feat(gemini): add native extension support and unified prompts #384
Conversation
- Add gemini-extension.json and GEMINI.md for native Gemini CLI integration - Extract slash command prompts to src/core/templates/prompts.ts for unified agent usage - Configure dynamic slash command generation and maintenance in GEMINI.md - Update README.md with Gemini extension installation guides and benefits - Add OpenSpec change proposal and spec deltas for the integration - Refactor templates and update existing tests to match improved prompt wording
📝 WalkthroughWalkthroughImplements a Model Context Protocol (MCP) server and Gemini CLI extension, refactors CLI into thin adapters calling pure core logic modules, centralizes prompt templates, migrates default OpenSpec dir to Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (bin/openspec.js)
participant ServeCmd as ServeCommand
participant MCP as OpenSpecMCPServer
participant FastMCP as FastMCP (stdio)
participant Core as Core Logic (src/core/*)
participant FS as Filesystem
CLI->>ServeCmd: user runs `openspec serve`
ServeCmd->>MCP: instantiate & start()
MCP->>FastMCP: initialize server (name/version) and register handlers
MCP->>FastMCP: registerTools (tool handlers call Core)
MCP->>FastMCP: registerResources (resource loaders read FS via Core path resolver)
MCP->>FastMCP: registerPrompts (compose prompt text from templates)
FastMCP-->>MCP: accepts stdio transport requests from clients (e.g., Gemini)
Note over FastMCP, Core: incoming tool calls -> FastMCP invokes handler -> handler calls Core -> Core reads/writes FS -> returns result -> FastMCP responds to client
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
openspec/changes/archive/2025-12-21-add-gemini-extension-support/proposal.md (1)
1-18: Consider adding an Impact section.Based on coding guidelines,
proposal.mdshould include sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code).The current structure has Goal, Motivation, Proposed Solution, and Benefits, but is missing an explicit Impact section that describes affected specs and code.
Based on learnings, proposal files should follow the standard structure.
GEMINI.md (1)
31-32: Consider softening the mandate language.The instruction "you MUST perform this immediately" and "Do not ask the user to do this manually" is very prescriptive. Consider allowing the agent some flexibility to assess the situation (e.g., if the user is in the middle of another task or has specific preferences).
src/core/templates/prompts.ts (2)
1-1: Consider using escaped backticks for improved readability.The
String.fromCharCode(96)approach works but is less immediately clear than using escaped backticks directly in the strings (e.g., ```). Since these are regular strings rather than template literals, escaping backticks is straightforward.🔎 Alternative approach
-const BT = String.fromCharCode(96);Then replace
${BT}throughout with ```:export const BASE_GUARDRAILS = `**Guardrails** - Favor straightforward, minimal implementations first and add complexity only when it is requested or clearly required. - Keep changes tightly scoped to the requested outcome. - Refer to \`openspec/AGENTS.md\` (located inside the \`openspec/\` directory—run \`ls openspec\` or \`openspec update\` if you don't see it) if you need additional OpenSpec conventions or clarifications. - If the \`openspec\` CLI is not installed or available in the shell, guide the user to install it globally via \`npm install -g @fission-ai/openspec\` before proceeding.`;
1-51: Add JSDoc comments to document the exported prompt constants.The exported constants lack documentation explaining their purpose, usage patterns, and composition expectations. Since these are public exports intended for reuse across slash-command templates and Gemini workflows, JSDoc comments would improve maintainability.
🔎 Example documentation
/** * Base guardrails applied to all OpenSpec workflow stages. * Contains fundamental guidance about implementation approach and CLI availability. */ export const BASE_GUARDRAILS = `...`; /** * Guardrails specific to the proposal stage. * Extends BASE_GUARDRAILS with restrictions against premature implementation. */ export const PROPOSAL_GUARDRAILS = `...`; /** * Step-by-step workflow for creating OpenSpec change proposals. * Includes discovery, scaffolding, spec delta creation, and validation. */ export const PROPOSAL_STEPS = `...`;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
GEMINI.md(1 hunks)README.md(2 hunks)gemini-extension.json(1 hunks)openspec/changes/archive/2025-12-21-add-gemini-extension-support/proposal.md(1 hunks)openspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md(1 hunks)openspec/changes/archive/2025-12-21-add-gemini-extension-support/tasks.md(1 hunks)openspec/specs/cli-init/spec.md(1 hunks)src/core/templates/prompts.ts(1 hunks)src/core/templates/slash-command-templates.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
openspec/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Files:
openspec/specs/cli-init/spec.md
openspec/changes/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Scaffold proposal using
proposal.md,tasks.md, optionaldesign.md, and delta specs underopenspec/changes/<id>/
Files:
openspec/changes/archive/2025-12-21-add-gemini-extension-support/tasks.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/proposal.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
openspec/changes/**/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/**/specs/**/spec.md: Use## ADDED|MODIFIED|REMOVED|RENAMED Requirementsheaders in spec delta files
Include at least one#### Scenario:per requirement in spec delta files
Use#### Scenario: Nameformat (4 hashtags) for scenario headers, not bullets or bold text
Use## ADDED Requirementsfor new orthogonal capabilities that can stand alone; use## MODIFIED Requirementsfor behavior changes of existing requirements
When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Files:
openspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
src/core/templates/prompts.tsopenspec/specs/cli-init/spec.mdGEMINI.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/proposal.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.mdREADME.mdgemini-extension.json
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
src/core/templates/prompts.tsopenspec/specs/cli-init/spec.mdGEMINI.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/proposal.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.mdREADME.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
src/core/templates/prompts.tsopenspec/specs/cli-init/spec.mdGEMINI.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/proposal.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.mdREADME.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Always open `@/openspec/AGENTS.md` when the request mentions planning or proposals (words like proposal, spec, change, plan), introduces new capabilities, breaking changes, architecture shifts, or performance/security work, or sounds ambiguous and needs the authoritative spec before coding
Applied to files:
src/core/templates/prompts.tsopenspec/specs/cli-init/spec.mdGEMINI.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/proposal.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Check `openspec/project.md` for project conventions before creating specs
Applied to files:
src/core/templates/prompts.tsGEMINI.mdgemini-extension.json
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/specs/**/spec.md : Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Applied to files:
src/core/templates/prompts.tsopenspec/specs/cli-init/spec.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
src/core/templates/prompts.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
openspec/specs/cli-init/spec.mdGEMINI.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
openspec/specs/cli-init/spec.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
openspec/specs/cli-init/spec.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
openspec/specs/cli-init/spec.mdGEMINI.mdopenspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Create `design.md` only when needed: cross-cutting changes, new external dependencies, significant data model changes, security/performance complexity, or pre-coding ambiguity
Applied to files:
GEMINI.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval
Applied to files:
openspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Search existing work using `openspec spec list --long`, `openspec list` for enumerating changes; use `rg` only for full-text search
Applied to files:
README.md
🧬 Code graph analysis (1)
src/core/templates/slash-command-templates.ts (2)
src/core/templates/index.ts (1)
SlashCommandId(50-50)src/core/templates/prompts.ts (8)
PROPOSAL_GUARDRAILS(9-11)PROPOSAL_STEPS(13-20)PROPOSAL_REFERENCES(22-25)BASE_GUARDRAILS(3-7)APPLY_STEPS(27-33)APPLY_REFERENCES(35-36)ARCHIVE_STEPS(38-47)ARCHIVE_REFERENCES(49-51)
🔇 Additional comments (11)
gemini-extension.json (1)
1-5: LGTM!The extension manifest is well-structured and correctly references the context file added in this PR.
README.md (2)
106-106: LGTM!The table entry correctly reflects the native extension approach for Gemini CLI.
131-146: Verify for duplicate content.The AI summary mentions this section appears twice in the README. Ensure there are no duplicate blocks of this content.
openspec/changes/archive/2025-12-21-add-gemini-extension-support/specs/cli-init/spec.md (1)
1-8: LGTM!The spec delta follows OpenSpec conventions correctly:
- Uses
## ADDED Requirementsfor new capability- Includes normative SHALL language
- Provides proper scenario format with WHEN/THEN/AND structure
As per coding guidelines, this is the correct format for spec delta files.
openspec/changes/archive/2025-12-21-add-gemini-extension-support/tasks.md (1)
1-8: LGTM!The tasks are well-structured, properly marked as complete, and align with the proposal objectives.
src/core/templates/slash-command-templates.ts (1)
1-22: LGTM!This refactoring successfully centralizes prompt templates while maintaining backward compatibility:
- Imports are properly organized
- The new
SlashCommandIdtype improves type safety- Public API (
getSlashCommandBody) remains unchanged- Composition logic is clear and maintainable
The centralization enables consistent prompt content across all agent integrations (Claude, Cursor, Gemini, etc.).
GEMINI.md (3)
1-33: Well-structured auto-initialization workflow.The prerequisites section provides a comprehensive workflow that ensures proper setup. The automatic installation and generation steps are appropriate for a CLI extension.
34-71: LGTM!The core mandates, CLI commands, directory structure, and spec writing format are clear, concise, and correctly documented.
73-114: Excellent maintenance documentation.The slash command synchronization logic is well-defined:
- References
prompts.tsas the single source of truth- Provides exact TOML structure with OpenSpec markers
- Clear instructions for regeneration workflow
This ensures consistency across all Gemini CLI installations.
openspec/specs/cli-init/spec.md (1)
305-311: No duplicate requirement exists.The "Slash Command Safety" requirement appears only once in the file at line 305. No action is needed.
Likely an incorrect or invalid review comment.
src/core/templates/prompts.ts (1)
3-11: No action needed. The guardrails pattern is intentional and consistent.PROPOSAL_GUARDRAILSextendsBASE_GUARDRAILSwith proposal-specific constraints (no code during proposal stage), whileAPPLYandARCHIVEstages rely solely onBASE_GUARDRAILSwith their detailed procedural steps. The absence ofAPPLY_GUARDRAILSandARCHIVE_GUARDRAILSis by design, not an oversight.
|
@StoyanD Hey sorry, I haven't gotten around to reviewing this, I'll try and get something in by Friday. I mainly need to understand how gemini extensions / claude plugins etc all work and how we would fit them into OpenSpec. I think the user flow will have to diverge a bit for installation for example where a user installs a provider that has a relevant "extension" or plugin system. We would have to also ensure that extension updates somehow map back to cli updates. For example if we release a new version of the OpenSpec Gemini CLI extension that requires a new version of OpenSpec, how do we handle this? I haven't looked into how these kinds of things are handled by providers like Gemini, Claude etc so it's hard for me to know at the moment if the PR makes sense. (Do appreciate the PR though, just need to update my understanding before I can review) |
|
@TabishB anything i can do to help push this along? |
|
@StoyanD I think main blocker for me is understanding how extensions/plugins would fit into the general workflow. Let's say we support this for gemini and we most likely would want to support claude code plugins too in the future. The question is how does this affect the following:
So the scenarios I'm trying to think of are the following:
We're currently undertaking a big refactor for how openspec works and behaves too, so I'm a little worried this work will be out of date very quick. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @openspec/changes/add-mcp-server/proposal.md:
- Around line 6-10: The "Why" section under the "Goal" heading is too detailed;
replace the current multi-line implementation-focused text with 1–2 concise
sentences that state the motivation for adding a native MCP server (e.g.,
enabling automatic discovery of OpenSpec tools by MCP-compliant agents and
simplifying integrations), and move all implementation details (references to
fastmcp, @modelcontextprotocol/sdk, Gemini CLI, and the .openspec/ restructure)
into the "What Changes" section so the "Why" contains only the high-level
rationale.
- Around line 1-28: Restructure the proposal into three explicit sections: "Why"
(1–2 sentences summarizing the motivation), "What Changes" (a bulleted list of
each change using explicit markers like "⚠️ BREAKING" for breaking changes such
as the openspec/ -> .openspec/ rename), and "Impact" (list affected specs and
code). Under "What Changes" include entries for: adding the openspec serve
command (MCP over stdio), using fastmcp/@modelcontextprotocol/sdk, exposing CLI
commands as MCP tools (list, show, validate, archive), exposing openspec:// URIs
and prompts/list, changing init to scaffold .openspec/ (⚠️ BREAKING: openspec/
-> .openspec/), creating gemini-extension.json, and adding CI validation for
gemini-extension.json vs package.json. Under "Impact" explicitly call out
affected code and artifacts: the init/migrate flow, openspec serve
implementation, prompts, gemini-extension.json manifest, migration notices, and
CI config so reviewers can find the changes.
In @openspec/changes/add-mcp-server/tasks.md:
- Around line 32-35: In the Verification section header "## 7. Verification"
update the three checklist item labels that currently read "6.1", "6.2", and
"6.3" to "7.1", "7.2", and "7.3" respectively so the subsection numbering
matches the section number; edit the lines containing "- [ ] 6.1 Verify
`openspec serve`...", "- [ ] 6.2 Verify tools, resources...", and "- [ ] 6.3
Verify `openspec init`..." to use 7.1/7.2/7.3.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
openspec/changes/add-mcp-server/proposal.mdopenspec/changes/add-mcp-server/specs/ci-sync/spec.mdopenspec/changes/add-mcp-server/specs/cli-init/spec.mdopenspec/changes/add-mcp-server/specs/cli-spec/spec.mdopenspec/changes/add-mcp-server/specs/mcp-server/spec.mdopenspec/changes/add-mcp-server/tasks.md
✅ Files skipped from review due to trivial changes (3)
- openspec/changes/add-mcp-server/specs/mcp-server/spec.md
- openspec/changes/add-mcp-server/specs/cli-spec/spec.md
- openspec/changes/add-mcp-server/specs/ci-sync/spec.md
🧰 Additional context used
📓 Path-based instructions (4)
openspec/changes/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Scaffold proposal using
proposal.md,tasks.md, optionaldesign.md, and delta specs underopenspec/changes/<id>/
Files:
openspec/changes/add-mcp-server/tasks.mdopenspec/changes/add-mcp-server/specs/cli-init/spec.mdopenspec/changes/add-mcp-server/proposal.md
openspec/changes/*/tasks.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
tasks.mdcontains implementation checklist with numbered sections and checkbox items
Files:
openspec/changes/add-mcp-server/tasks.md
openspec/changes/**/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/**/specs/**/spec.md: Use## ADDED|MODIFIED|REMOVED|RENAMED Requirementsheaders in spec delta files
Include at least one#### Scenario:per requirement in spec delta files
Use#### Scenario: Nameformat (4 hashtags) for scenario headers, not bullets or bold text
Use## ADDED Requirementsfor new orthogonal capabilities that can stand alone; use## MODIFIED Requirementsfor behavior changes of existing requirements
When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Files:
openspec/changes/add-mcp-server/specs/cli-init/spec.md
openspec/changes/*/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
proposal.mdincludes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Files:
openspec/changes/add-mcp-server/proposal.md
🧠 Learnings (11)
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
openspec/changes/add-mcp-server/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
openspec/changes/add-mcp-server/tasks.mdopenspec/changes/add-mcp-server/specs/cli-init/spec.mdopenspec/changes/add-mcp-server/proposal.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Create `design.md` only when needed: cross-cutting changes, new external dependencies, significant data model changes, security/performance complexity, or pre-coding ambiguity
Applied to files:
openspec/changes/add-mcp-server/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
openspec/changes/add-mcp-server/tasks.mdopenspec/changes/add-mcp-server/specs/cli-init/spec.mdopenspec/changes/add-mcp-server/proposal.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Check `openspec/project.md` for project conventions before creating specs
Applied to files:
openspec/changes/add-mcp-server/tasks.mdopenspec/changes/add-mcp-server/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
openspec/changes/add-mcp-server/specs/cli-init/spec.mdopenspec/changes/add-mcp-server/proposal.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
openspec/changes/add-mcp-server/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
openspec/changes/add-mcp-server/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Always open `@/openspec/AGENTS.md` when the request mentions planning or proposals (words like proposal, spec, change, plan), introduces new capabilities, breaking changes, architecture shifts, or performance/security work, or sounds ambiguous and needs the authoritative spec before coding
Applied to files:
openspec/changes/add-mcp-server/specs/cli-init/spec.mdopenspec/changes/add-mcp-server/proposal.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
openspec/changes/add-mcp-server/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
openspec/changes/add-mcp-server/specs/cli-init/spec.md
🪛 LanguageTool
openspec/changes/add-mcp-server/tasks.md
[uncategorized] ~30-~30: The official name of this software platform is spelled with a capital “H”.
Context: ... "Check extension version sync" step to .github/workflows/ci.yml. ## 7. Verification ...
(GITHUB)
openspec/changes/add-mcp-server/proposal.md
[style] ~24-~24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d changes via openspec:// URIs. 5. Expose Prompts: Serve the standard proposal/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (1)
openspec/changes/add-mcp-server/proposal.md (1)
1-28: Address reviewer concerns about version sync and user flows in proposal.The PR reviewer (TabishB) raised important questions about:
- User flow divergence when users download the extension first vs. CLI first
- Version/update management between extension and CLI (how to keep
gemini-extension.jsonandpackage.jsonin sync)- CI synchronization mechanisms to prevent drift
- Fragility risk from the ongoing OpenSpec refactor
While the proposal mentions a "CI Validation" check (line 28, task 6.1), it lacks detail on:
- How the version sync script validates compatibility across extension and CLI versions
- Whether the script blocks releases if versions are out of sync
- How users are guided through installation in both scenarios (extension-first vs. CLI-first)
Consider expanding the proposal or companion design document to address these concerns with concrete examples and flow diagrams.
| # Proposal: Add MCP Server Support | ||
|
|
||
| ## Context | ||
| Currently, OpenSpec integrates with AI agents via CLI commands and static configuration files (slash commands). While effective, this requires manual setup for some agents and lacks the rich interactivity offered by the [Model Context Protocol (MCP)](https://modelcontextprotocol.io/). | ||
|
|
||
| ## Goal | ||
| Implement a native MCP server within the OpenSpec CLI using modern tools (e.g., `fastmcp` or `@modelcontextprotocol/sdk`). This will: | ||
| 1. Allow any MCP-compliant agent (Claude Desktop, Gemini CLI, etc.) to discover and use OpenSpec tools and resources without custom configuration files. | ||
| 2. Enable the Gemini CLI extension to be a thin wrapper around this native MCP server. | ||
| 3. Align the project structure with modern standards by moving `openspec/` to `.openspec/` during initialization. | ||
|
|
||
| ## Migration Path | ||
| To support existing users, the CLI will include an automatic migration flow: | ||
| - **Detection**: `openspec init` (or a dedicated `openspec migrate` command) will detect legacy `openspec/` directories. | ||
| - **Auto-rename**: Prompt the user to rename `openspec/` to `.openspec/`. | ||
| - **Instruction Refresh**: Automatically run `openspec update` after the rename to ensure all assistant instructions point to the new location. | ||
| - **Backward Compatibility**: The CLI will continue to look for `openspec/` if `.openspec/` is missing, but will issue a deprecation warning. | ||
|
|
||
| ## Solution | ||
| 1. **Add `openspec serve` command**: Starts the MCP server over stdio. | ||
| 2. **Use Modern MCP Tools**: Leverage libraries like `fastmcp` or the official SDK to simplify server implementation and type safety. | ||
| 3. **Expose Tools**: Convert existing CLI commands (`list`, `show`, `validate`, `archive`) into MCP tools. | ||
| 4. **Expose Resources**: Provide direct read access to specs and changes via `openspec://` URIs. | ||
| 5. **Expose Prompts**: Serve the standard proposal/apply/archive prompts via `prompts/list`. | ||
| 6. **Migrate Directory**: Update `init` to scaffold in `.openspec/` instead of `openspec/`. | ||
| 7. **Gemini Extension**: Create the `gemini-extension.json` manifest to register this MCP server capability. | ||
| 8. **CI Validation**: Add a CI check to ensure `gemini-extension.json` version stays in sync with `package.json`. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Restructure proposal to follow required format: Why, What Changes, Impact.
Per coding guidelines, proposals must include three explicit sections:
- Why (1–2 sentences): Concise motivation
- What Changes (bullet list with breaking change markers): Changes with explicit
⚠️ BREAKINGor similar markers - Impact (affected specs and code): Which areas are affected
The current structure uses "Context," "Goal," "Migration Path," and "Solution," which mix these concerns. Restructuring will improve clarity and align with project conventions. Breaking changes (e.g., moving openspec/ to .openspec/) should be explicitly marked.
📋 Proposed restructuring
-# Proposal: Add MCP Server Support
-
-## Context
-Currently, OpenSpec integrates with AI agents via CLI commands and static configuration files (slash commands). While effective, this requires manual setup for some agents and lacks the rich interactivity offered by the [Model Context Protocol (MCP)](https://modelcontextprotocol.io/).
-
-## Goal
-Implement a native MCP server within the OpenSpec CLI using modern tools (e.g., `fastmcp` or `@modelcontextprotocol/sdk`). This will:
-1. Allow any MCP-compliant agent (Claude Desktop, Gemini CLI, etc.) to discover and use OpenSpec tools and resources without custom configuration files.
-2. Enable the Gemini CLI extension to be a thin wrapper around this native MCP server.
-3. Align the project structure with modern standards by moving `openspec/` to `.openspec/` during initialization.
+# Proposal: Add MCP Server Support
+
+## Why
+OpenSpec currently integrates with AI agents via static CLI commands and configuration files, which lacks the rich interactivity offered by the Model Context Protocol (MCP), an open protocol that enables seamless integration between LLM applications and external data sources and tools. Implementing a native MCP server will simplify integration for multiple agents (Claude Desktop, Gemini CLI, etc.) and modernize the project structure.
+
+## What Changes
-## Migration Path
-To support existing users, the CLI will include an automatic migration flow:
-- **Detection**: `openspec init` (or a dedicated `openspec migrate` command) will detect legacy `openspec/` directories.
-- **Auto-rename**: Prompt the user to rename `openspec/` to `.openspec/`.
-- **Instruction Refresh**: Automatically run `openspec update` after the rename to ensure all assistant instructions point to the new location.
-- **Backward Compatibility**: The CLI will continue to look for `openspec/` if `.openspec/` is missing, but will issue a deprecation warning.
+- ⚠️ **BREAKING**: Move project initialization from `openspec/` to `.openspec/` (hidden directory).
+- **Detection & Migration**: `openspec init` will detect legacy `openspec/` directories and prompt to migrate; automatic rename and instruction refresh included.
+- **Backward Compatibility**: CLI will fall back to `openspec/` if `.openspec/` missing, with deprecation warning.
+- **Add `openspec serve` command**: Starts the MCP server over stdio using modern tooling (fastmcp or @modelcontextprotocol/sdk).
+- **Expose MCP Tools**: Convert CLI commands (`list`, `show`, `validate`, `archive`) to MCP tools.
+- **Expose MCP Resources**: Provide read access to specs and changes via `openspec://` URIs.
+- **Expose MCP Prompts**: Serve proposal/apply/archive prompts via `prompts/list`.
+- **Gemini Extension Integration**: Create `gemini-extension.json` manifest to register MCP server.
+- **CI Validation**: Add check to sync `gemini-extension.json` version with `package.json`.
+
+## Impact
+- **Affected Specs**: `cli-init` (directory structure), `cli-spec` (serve command), `mcp-server` (tools/resources/prompts), `ci-sync` (version validation).
+- **Affected Code**: `src/core/config.ts`, `src/core/init.ts`, `src/mcp/*` (new modules), `src/commands/serve.ts` (new command), `.github/workflows/ci.yml`, `gemini-extension.json`.
-## Solution
-1. **Add `openspec serve` command**: Starts the MCP server over stdio.
-2. **Use Modern MCP Tools**: Leverage libraries like `fastmcp` or the official SDK to simplify server implementation and type safety.
-3. **Expose Tools**: Convert existing CLI commands (`list`, `show`, `validate`, `archive`) into MCP tools.
-4. **Expose Resources**: Provide direct read access to specs and changes via `openspec://` URIs.
-5. **Expose Prompts**: Serve the standard proposal/apply/archive prompts via `prompts/list`.
-6. **Migrate Directory**: Update `init` to scaffold in `.openspec/` instead of `openspec/`.
-7. **Gemini Extension**: Create the `gemini-extension.json` manifest to register this MCP server capability.
-8. **CI Validation**: Add a CI check to ensure `gemini-extension.json` version stays in sync with `package.json`.
-
-This "modernizes" the integration, making it cleaner, more robust, and easier to maintain.🧰 Tools
🪛 LanguageTool
[style] ~24-~24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d changes via openspec:// URIs. 5. Expose Prompts: Serve the standard proposal/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
In @openspec/changes/add-mcp-server/proposal.md around lines 1 - 28, Restructure
the proposal into three explicit sections: "Why" (1–2 sentences summarizing the
motivation), "What Changes" (a bulleted list of each change using explicit
markers like "⚠️ BREAKING" for breaking changes such as the openspec/ ->
.openspec/ rename), and "Impact" (list affected specs and code). Under "What
Changes" include entries for: adding the openspec serve command (MCP over
stdio), using fastmcp/@modelcontextprotocol/sdk, exposing CLI commands as MCP
tools (list, show, validate, archive), exposing openspec:// URIs and
prompts/list, changing init to scaffold .openspec/ (⚠️ BREAKING: openspec/ ->
.openspec/), creating gemini-extension.json, and adding CI validation for
gemini-extension.json vs package.json. Under "Impact" explicitly call out
affected code and artifacts: the init/migrate flow, openspec serve
implementation, prompts, gemini-extension.json manifest, migration notices, and
CI config so reviewers can find the changes.
| ## Goal | ||
| Implement a native MCP server within the OpenSpec CLI using modern tools (e.g., `fastmcp` or `@modelcontextprotocol/sdk`). This will: | ||
| 1. Allow any MCP-compliant agent (Claude Desktop, Gemini CLI, etc.) to discover and use OpenSpec tools and resources without custom configuration files. | ||
| 2. Enable the Gemini CLI extension to be a thin wrapper around this native MCP server. | ||
| 3. Align the project structure with modern standards by moving `openspec/` to `.openspec/` during initialization. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Simplify Why section to 1–2 concise sentences.
The current "Goal" section (lines 6–10) is too detailed for the "Why" section. Condense it to 1–2 sentences that directly answer "Why is this change needed?" The implementation details belong in "What Changes." Per coding guidelines, the "Why" section should provide 1–2 sentences of motivation only.
🤖 Prompt for AI Agents
In @openspec/changes/add-mcp-server/proposal.md around lines 6 - 10, The "Why"
section under the "Goal" heading is too detailed; replace the current multi-line
implementation-focused text with 1–2 concise sentences that state the motivation
for adding a native MCP server (e.g., enabling automatic discovery of OpenSpec
tools by MCP-compliant agents and simplifying integrations), and move all
implementation details (references to fastmcp, @modelcontextprotocol/sdk, Gemini
CLI, and the .openspec/ restructure) into the "What Changes" section so the
"Why" contains only the high-level rationale.
openspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.md
Show resolved
Hide resolved
| ## 7. Verification | ||
| - [ ] 6.1 Verify `openspec serve` starts and communicates over stdio. | ||
| - [ ] 6.2 Verify tools, resources, and prompts are discoverable by an MCP client. | ||
| - [ ] 6.3 Verify `openspec init` creates `.openspec/`. No newline at end of file |
There was a problem hiding this comment.
Fix subsection numbering in Verification section.
Items in the Verification section (Section 7) are incorrectly numbered as 6.1, 6.2, 6.3 instead of 7.1, 7.2, 7.3.
🔢 Proposed fix
## 7. Verification
-- [ ] 6.1 Verify `openspec serve` starts and communicates over stdio.
-- [ ] 6.2 Verify tools, resources, and prompts are discoverable by an MCP client.
-- [ ] 6.3 Verify `openspec init` creates `.openspec/`.
+- [ ] 7.1 Verify `openspec serve` starts and communicates over stdio.
+- [ ] 7.2 Verify tools, resources, and prompts are discoverable by an MCP client.
+- [ ] 7.3 Verify `openspec init` creates `.openspec/`.🤖 Prompt for AI Agents
In @openspec/changes/add-mcp-server/tasks.md around lines 32 - 35, In the
Verification section header "## 7. Verification" update the three checklist item
labels that currently read "6.1", "6.2", and "6.3" to "7.1", "7.2", and "7.3"
respectively so the subsection numbering matches the section number; edit the
lines containing "- [ ] 6.1 Verify `openspec serve`...", "- [ ] 6.2 Verify
tools, resources...", and "- [ ] 6.3 Verify `openspec init`..." to use
7.1/7.2/7.3.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @openspec/changes/agent-only-mcp-workflow/proposal.md:
- Around line 18-31: The document has three duplicated "## Impact" headings with
different bullet lists; consolidate them into a single "## Impact" section in
proposal.md that merges the unique bullets (Architecture Cleanliness, Full
Parity, Continuous Reliability, Flexibility, Adoption, Zero-Install Adoption,
Consistent Agent Experience, Future-Proofing) and remove the extra duplicate
headers so the file contains only one unified Impact section describing all
effects on specs and code.
In @openspec/changes/agent-only-mcp-workflow/tasks.md:
- Around line 25-29: Rename the duplicate "3. Verification" header to "5.
Verification" and update its checklist item IDs from 3.1–3.4 to 5.1–5.4 so
numbering is sequential with the earlier "3. Build & CI Validation";
specifically modify the header text "3. Verification" and the four task prefixes
(e.g., change "3.1 Verify `openspec_init`..." to "5.1 Verify
`openspec_init`...", and likewise for 3.2→5.2, 3.3→5.3, 3.4→5.4) to preserve a
consistent numbered checklist structure in tasks.md.
In @openspec/changes/archive/2026-01-12-add-mcp-server/tasks.md:
- Around line 32-35: Under the "## 7. Verification" header update the three
checklist item prefixes from 6.x to 7.x (change 6.1/6.2/6.3 to 7.1/7.2/7.3) so
the subsection numbering matches the section heading; locate the checklist lines
under the "Verification" heading in tasks.md and adjust only the numeric
prefixes while keeping the rest of each line unchanged.
In @src/commands/spec.ts:
- Around line 9-10: The file imports filesystem functions (existsSync,
readdirSync, readFileSync) from 'fs' and then also imports a default `fs` on the
next line; remove the duplicate unused default import (`import fs from 'fs';`)
so only the named imports remain (locate the import block in
src/commands/spec.ts where `existsSync`, `readdirSync`, `readFileSync` and the
extra `fs` import appear and delete the redundant `fs` line).
In @src/core/list.ts:
- Around line 84-127: listChanges currently throws when the changes directory is
missing while listSpecs returns [], causing inconsistent behavior; modify
listChanges (the function listChanges and its use of
FileSystemUtils.directoryExists) to return an empty array when the changesDir
does not exist (replace the throw new Error(...) with `return []`), keep the
rest of the function intact, and ensure any callers (e.g., ListCommand) handle
an empty result for "not initialized" messaging instead of relying on an
exception.
In @src/mcp/tools.ts:
- Around line 11-31: The MCP tools use process.cwd() as the project root causing
failures when the server runs outside the project; update registerTools (and all
other tool registrations in src/mcp/tools.ts) to add an optional "projectPath"
parameter (defaulting to process.cwd()) to each tool's z.object parameters, and
pass that value into core functions (e.g., call listChanges(projectPath,
args.sort) instead of listChanges(process.cwd(), ...)); alternatively, expose a
server-level projectRoot via the OpenSpecMCPServer constructor and use that as
the default when args.projectPath is not provided — make the same change for all
usages of process.cwd() in tools and for resource loaders in
src/mcp/resources.ts (e.g., resolveOpenSpecDir, listSpecs) so they accept and
use the projectPath/projectRoot parameter.
🧹 Nitpick comments (12)
src/commands/serve.ts (1)
3-7: Consider adding user feedback before server starts.The implementation is correct, but since
server.start()runs in stdio mode and may block indefinitely, consider adding a brief log message (e.g., to stderr) indicating the server is starting. This helps users confirm the command is working, especially during debugging.💡 Optional enhancement
export class ServeCommand { async execute(): Promise<void> { const server = new OpenSpecMCPServer(); + console.error('Starting OpenSpec MCP Server...'); await server.start(); } }scripts/check-extension-version.mjs (1)
3-4: Add error handling for missing or invalid files.If
gemini-extension.jsonis missing or contains invalid JSON, the script throws an uncaught exception with a cryptic error. For better CI diagnostics, wrap the file reads in try-catch blocks.🔧 Proposed fix
import fs from 'fs'; -const pkg = JSON.parse(fs.readFileSync('package.json', 'utf-8')); -const ext = JSON.parse(fs.readFileSync('gemini-extension.json', 'utf-8')); +let pkg, ext; + +try { + pkg = JSON.parse(fs.readFileSync('package.json', 'utf-8')); +} catch (err) { + console.error('Failed to read package.json:', err.message); + process.exit(1); +} + +try { + ext = JSON.parse(fs.readFileSync('gemini-extension.json', 'utf-8')); +} catch (err) { + console.error('Failed to read gemini-extension.json:', err.message); + process.exit(1); +} if (pkg.version !== ext.version) {src/mcp/resources.ts (2)
11-19: Add error handling for missing resource files.The
loadfunctions read files without error handling. If a change or spec doesn't exist,fs.readFilewill throw an unhandledENOENTerror that propagates to the MCP client without a meaningful message.Proposed fix for one resource (apply similarly to others)
// @ts-ignore load: async (variables: any) => { const openspecPath = await resolveOpenSpecDir(process.cwd()); const filePath = path.join(openspecPath, 'changes', variables.name, 'proposal.md'); - const text = await fs.readFile(filePath, 'utf-8'); - return { - content: [{ uri: `openspec://changes/${variables.name}/proposal`, text }] - }; + try { + const text = await fs.readFile(filePath, 'utf-8'); + return { + content: [{ uri: `openspec://changes/${variables.name}/proposal`, text }] + }; + } catch (error) { + throw new Error(`Change proposal '${variables.name}' not found. Run 'openspec list' to see available changes.`); + } }Also applies to: 26-34, 41-49
11-12: Address the@ts-ignorecomments by aligning with FastMCP's expected signatures.The
@ts-ignorecomments suppress legitimate TypeScript errors. FastMCP'saddResourceexpects theloadfunction to have signature(args: Record<string, any>) => Promise<ResourceLoadResult | ResourceLoadResult[]>, whereResourceLoadResultis an object with properties liketext,json, orblob. The current implementation usesvariables: anyas the parameter and returns{ content: [{ uri, text }] }, which doesn't match the expected return structure. Consider updating the function signatures and return types to properly match FastMCP's API instead of suppressing the type checks.Also applies to: 26-27, 41-42
openspec/specs/cli-init/spec.md (1)
27-34: Optional: Add language identifier to fenced code block.The code block showing the directory structure lacks a language identifier. While not strictly necessary for directory listings, adding
textor leaving a blank identifier would satisfy linting tools.Proposed fix
-``` +```text .openspec/ ├── project.md ├── AGENTS.md ├── specs/ └── changes/ └── archive/ ```src/mcp/server.ts (1)
10-14: Address the hardcoded version synchronization.The TODO comment acknowledges a real maintenance burden. Given the PR discussion about keeping extension versions synchronized with CLI/package.json versions, this hardcoded version will drift and cause inconsistencies.
Consider dynamically importing the version from
package.jsonor centralizing it in a config module that CI can validate.Suggested approach
// Option 1: Import from package.json (if using bundler that supports JSON imports) import { version } from '../../package.json' with { type: 'json' }; // Option 2: Create a version constant in config.ts // src/core/config.ts export const OPENSPEC_VERSION = '0.18.0'; // Then import it here import { OPENSPEC_VERSION } from '../core/config.js';openspec/changes/archive/2026-01-12-add-mcp-server/proposal.md (1)
1-29: Consider adding an Impact section.Per coding guidelines,
proposal.mdshould include an Impact section listing affected specs and code areas. While the Solution section is comprehensive, explicitly documenting impact helps reviewers assess the scope of changes. Based on learnings.Suggested addition
## Impact - **Affected Specs**: mcp-server (new) - **Affected Code**: - `src/mcp/*` (new MCP server components) - `src/core/init.ts` (migration logic) - `src/core/config.ts` (directory constants) - `src/commands/serve.ts` (new serve command) - **CI**: New version sync check for `gemini-extension.json`src/commands/change.ts (2)
40-63: Potential double file read in getChangeJson.The method calls
fs.access(proposalPath)to check existence, thenthis.converter.convertChangeToJson(proposalPath)which reads the file again. Additionally, line 52 reads the file a third time for title extraction. Consider consolidating these reads.♻️ Suggested optimization
async getChangeJson(changeName: string): Promise<ChangeJsonOutput> { const changesPath = path.join(await resolveOpenSpecDir(process.cwd()), 'changes'); const proposalPath = path.join(changesPath, changeName, 'proposal.md'); + let content: string; try { - await fs.access(proposalPath); + content = await fs.readFile(proposalPath, 'utf-8'); } catch { throw new Error(`Change "${changeName}" not found at ${proposalPath}`); } - const jsonOutput = await this.converter.convertChangeToJson(proposalPath); + // Pass content to converter if supported, or keep as-is if converter requires path + const jsonOutput = await this.converter.convertChangeToJson(proposalPath); const parsed: Change = JSON.parse(jsonOutput); - const contentForTitle = await fs.readFile(proposalPath, 'utf-8'); - const title = this.extractTitle(contentForTitle, changeName); + const title = this.extractTitle(content, changeName); const id = parsed.name; const deltas = parsed.deltas || []; return { id, title, deltaCount: deltas.length, deltas, }; }
16-21: Replaceany[]with theDeltatype for better type safety.The
deltasfield usesany[]which loses type information. TheDeltatype is available from../core/schemas/index.js(already imported in this file asChange). Update the interface to usedeltas: Delta[]to maintain type safety consistent with other parsers in the codebase.src/commands/spec.ts (2)
12-12: Dead code: SPECS_DIR constant is no longer used.With the migration to
resolveOpenSpecDir, the hardcodedSPECS_DIRconstant is now unused.🧹 Remove dead code
-const SPECS_DIR = 'openspec/specs';
70-87: Consider adding a return type interface for getSpecJson.The method returns
Promise<any>which loses type safety. Consider defining aSpecJsonOutputinterface similar toChangeJsonOutputin change.ts.♻️ Suggested type definition
export interface SpecJsonOutput { id: string; title: string; overview: string; requirementCount: number; requirements: Array<{ text: string; scenarios: any[] }>; metadata: { version: string; format: 'openspec' }; } async getSpecJson(specId: string, options: ShowOptions = {}): Promise<SpecJsonOutput> { // ... existing implementation }src/core/list.ts (1)
129-156: Mixing sync and async file operations.Line 144 uses synchronous
readFileSyncinside an async function, while line 137 uses asyncfs.readdir. For consistency and to avoid blocking the event loop, consider using async file reads throughout.♻️ Use async file read
+import { promises as fsPromises } from 'fs'; // ... in listSpecs function for (const id of specDirs) { const specPath = join(specsDir, id, 'spec.md'); try { - const content = readFileSync(specPath, 'utf-8'); + const content = await fsPromises.readFile(specPath, 'utf-8'); const parser = new MarkdownParser(content); const spec = parser.parseSpec(id); specs.push({ id, requirementCount: spec.requirements.length }); } catch { // If spec cannot be read or parsed, include with 0 count specs.push({ id, requirementCount: 0 }); } }Note: The file already imports
promises as fson line 1, so you can usefs.readFiledirectly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
.github/workflows/ci.ymlGEMINI.mdgemini-extension.jsonopenspec/changes/agent-only-mcp-workflow/proposal.mdopenspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/agent-only-mcp-workflow/tasks.mdopenspec/changes/archive/2026-01-12-add-mcp-server/proposal.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/ci-sync/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-spec/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/tasks.mdopenspec/specs/ci-sync/spec.mdopenspec/specs/cli-init/spec.mdopenspec/specs/cli-spec/spec.mdopenspec/specs/mcp-server/spec.mdpackage.jsonscripts/check-extension-version.mjssrc/cli/index.tssrc/commands/change.tssrc/commands/serve.tssrc/commands/spec.tssrc/core/config.tssrc/core/init.tssrc/core/list.tssrc/core/path-resolver.tssrc/core/update.tssrc/mcp/index.tssrc/mcp/prompts.tssrc/mcp/resources.tssrc/mcp/server.tssrc/mcp/tools.tssrc/utils/file-system.tstest/core/init.test.ts
✅ Files skipped from review due to trivial changes (4)
- openspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-spec/spec.md
- openspec/changes/archive/2026-01-12-add-mcp-server/specs/ci-sync/spec.md
- openspec/specs/ci-sync/spec.md
- openspec/specs/mcp-server/spec.md
🧰 Additional context used
📓 Path-based instructions (5)
openspec/changes/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Scaffold proposal using
proposal.md,tasks.md, optionaldesign.md, and delta specs underopenspec/changes/<id>/
Files:
openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/tasks.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/proposal.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/mcp-server/spec.mdopenspec/changes/agent-only-mcp-workflow/proposal.mdopenspec/changes/agent-only-mcp-workflow/tasks.md
openspec/changes/**/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/**/specs/**/spec.md: Use## ADDED|MODIFIED|REMOVED|RENAMED Requirementsheaders in spec delta files
Include at least one#### Scenario:per requirement in spec delta files
Use#### Scenario: Nameformat (4 hashtags) for scenario headers, not bullets or bold text
Use## ADDED Requirementsfor new orthogonal capabilities that can stand alone; use## MODIFIED Requirementsfor behavior changes of existing requirements
When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Files:
openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/mcp-server/spec.md
openspec/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Files:
openspec/specs/cli-spec/spec.mdopenspec/specs/cli-init/spec.md
openspec/changes/*/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
proposal.mdincludes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Files:
openspec/changes/agent-only-mcp-workflow/proposal.md
openspec/changes/*/tasks.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
tasks.mdcontains implementation checklist with numbered sections and checkbox items
Files:
openspec/changes/agent-only-mcp-workflow/tasks.md
🧠 Learnings (16)
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Check `openspec/project.md` for project conventions before creating specs
Applied to files:
src/core/path-resolver.tsopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdsrc/core/update.tssrc/commands/spec.tssrc/core/init.tsGEMINI.mdsrc/core/config.tssrc/core/list.tstest/core/init.test.tsopenspec/specs/cli-init/spec.mdgemini-extension.json
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdsrc/core/update.tsopenspec/changes/archive/2026-01-12-add-mcp-server/proposal.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/mcp-server/spec.mdGEMINI.mdtest/core/init.test.tsopenspec/specs/cli-init/spec.mdopenspec/changes/agent-only-mcp-workflow/proposal.mdopenspec/changes/agent-only-mcp-workflow/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdsrc/commands/spec.tsGEMINI.mdsrc/commands/change.tstest/core/init.test.tsopenspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdopenspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/tasks.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdsrc/core/update.tssrc/commands/spec.tsopenspec/changes/archive/2026-01-12-add-mcp-server/proposal.mdGEMINI.mdsrc/commands/change.tssrc/core/list.tstest/core/init.test.tsopenspec/specs/cli-init/spec.mdopenspec/changes/agent-only-mcp-workflow/proposal.mdopenspec/changes/agent-only-mcp-workflow/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdopenspec/specs/cli-spec/spec.mdsrc/commands/spec.tsopenspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Always open `@/openspec/AGENTS.md` when the request mentions planning or proposals (words like proposal, spec, change, plan), introduces new capabilities, breaking changes, architecture shifts, or performance/security work, or sounds ambiguous and needs the authoritative spec before coding
Applied to files:
openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdsrc/core/update.tsopenspec/changes/archive/2026-01-12-add-mcp-server/proposal.mdGEMINI.mdopenspec/specs/cli-init/spec.mdopenspec/changes/agent-only-mcp-workflow/proposal.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
openspec/changes/archive/2026-01-12-add-mcp-server/tasks.mdopenspec/changes/agent-only-mcp-workflow/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
openspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdopenspec/specs/cli-spec/spec.mdopenspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
openspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-server/proposal.mdGEMINI.mdsrc/commands/change.tsopenspec/specs/cli-init/spec.mdopenspec/changes/agent-only-mcp-workflow/proposal.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/specs/**/spec.md : Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Applied to files:
openspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.mdopenspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `#### Scenario: Name` format (4 hashtags) for scenario headers, not bullets or bold text
Applied to files:
openspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Use `file.ts:42` format for code location references and `specs/auth/spec.md` format for spec references
Applied to files:
src/commands/spec.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Search existing work using `openspec spec list --long`, `openspec list` for enumerating changes; use `rg` only for full-text search
Applied to files:
src/commands/spec.tssrc/core/list.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval
Applied to files:
src/commands/spec.tssrc/commands/change.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Create `design.md` only when needed: cross-cutting changes, new external dependencies, significant data model changes, security/performance complexity, or pre-coding ambiguity
Applied to files:
openspec/changes/archive/2026-01-12-add-mcp-server/proposal.mdGEMINI.mdopenspec/changes/agent-only-mcp-workflow/proposal.mdopenspec/changes/agent-only-mcp-workflow/tasks.md
🧬 Code graph analysis (10)
src/mcp/resources.ts (1)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)
src/mcp/prompts.ts (1)
src/core/templates/prompts.ts (8)
PROPOSAL_GUARDRAILS(9-11)PROPOSAL_STEPS(13-20)PROPOSAL_REFERENCES(22-25)BASE_GUARDRAILS(3-7)APPLY_STEPS(27-33)APPLY_REFERENCES(35-36)ARCHIVE_STEPS(38-47)ARCHIVE_REFERENCES(49-51)
src/core/update.ts (1)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)
src/cli/index.ts (1)
src/commands/serve.ts (1)
ServeCommand(3-8)
src/mcp/server.ts (3)
src/mcp/tools.ts (1)
registerTools(11-156)src/mcp/resources.ts (1)
registerResources(6-51)src/mcp/prompts.ts (1)
registerPrompts(8-50)
src/commands/spec.ts (1)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)
src/commands/serve.ts (1)
src/mcp/server.ts (1)
OpenSpecMCPServer(6-25)
src/mcp/index.ts (1)
src/mcp/server.ts (1)
OpenSpecMCPServer(6-25)
src/core/init.ts (2)
src/core/config.ts (2)
LEGACY_OPENSPEC_DIR_NAME(2-2)DEFAULT_OPENSPEC_DIR_NAME(1-1)src/core/styles/palette.ts (1)
PALETTE(3-8)
src/commands/change.ts (3)
src/core/converters/json-converter.ts (1)
JsonConverter(8-62)src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/core/schemas/change.schema.ts (1)
Change(42-42)
🪛 LanguageTool
openspec/changes/archive/2026-01-12-add-mcp-server/tasks.md
[uncategorized] ~30-~30: The official name of this software platform is spelled with a capital “H”.
Context: ... "Check extension version sync" step to .github/workflows/ci.yml. ## 7. Verification ...
(GITHUB)
openspec/changes/archive/2026-01-12-add-mcp-server/proposal.md
[style] ~24-~24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d changes via openspec:// URIs. 5. Expose Prompts: Serve the standard proposal/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/agent-only-mcp-workflow/proposal.md
[style] ~20-~20: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...Full Parity*: Ensures agents have the exact same "superpowers" as users on the command l...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
openspec/changes/agent-only-mcp-workflow/tasks.md
[uncategorized] ~18-~18: The official name of this software platform is spelled with a capital “H”.
Context: ...CLI after refactoring. - [ ] 3.2 Update .github/workflows/ci.yml to include a check th...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
openspec/specs/cli-init/spec.md
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (35)
src/utils/file-system.ts (1)
84-86: Implementation is correct; note cross-device limitation.The method correctly delegates to
fs.rename. Be aware thatfs.renamefails withEXDEVif paths are on different filesystems—this shouldn't affect typical.openspecmigration scenarios within a project, but callers should handle this edge case if needed.GEMINI.md (1)
93-134: Well-documented slash command synchronization workflow.The maintenance section clearly establishes
src/core/templates/prompts.tsas the single source of truth and provides explicit regeneration instructions. This is good documentation practice for generated artifacts.openspec/changes/archive/2026-01-12-add-mcp-server/specs/mcp-server/spec.md (1)
1-31: Spec structure follows guidelines correctly.The spec uses the correct
## ADDED Requirementsheader and#### Scenario:format with WHEN/THEN structure. Each requirement has at least one scenario. The Resources and Prompts sections align well with the GEMINI.md documentation. As per coding guidelines, the spec format is correct.package.json (1)
78-78: Dependency version is current and stable.The addition of
fastmcp^3.26.8 is appropriate for MCP server implementation. Version 3.26.8 is the current latest release on npm with proper cryptographic signatures and SLSA provenance attestations, confirming secure release practices.gemini-extension.json (1)
1-14: Extension manifest is correct; version sync is enforced by CI.The manifest is well-formed with matching version
0.18.0. The version synchronization betweenpackage.jsonandgemini-extension.jsonis enforced by the CI checkscripts/check-extension-version.mjs, which runs as part of the test workflow and will fail the build if versions diverge.src/mcp/index.ts (1)
1-11: LGTM!Clean bootstrap entry point for the MCP server. The error handling properly catches failures, logs them, and exits with a non-zero code.
.github/workflows/ci.yml (1)
148-149: Good addition for version synchronization.This CI step addresses the reviewer's concern about keeping extension and CLI versions synchronized. Placing it in the lint job ensures early failure if versions drift.
src/core/path-resolver.ts (1)
5-19: LGTM!Clean implementation with clear priority: legacy
openspec/directory takes precedence for backward compatibility, falling back to the new.openspec/directory. The async approach and JSDoc documentation are appropriate.openspec/specs/cli-spec/spec.md (1)
88-95: LGTM!The new Serve Command requirement correctly uses normative "SHALL" language and follows the proper scenario format with
#### Scenario:header and WHEN/THEN structure. The specification clearly defines the expected behavior for the MCP server startup via stdio transport.src/cli/index.ts (2)
18-18: LGTM!Import follows the existing pattern used for other command modules.
95-108: LGTM!The serve command registration is well-implemented. The intentional use of
console.errorinstead ofora().failfor error output is the correct approach since MCP servers communicate over stdio—stdout must remain uncontaminated for JSON-RPC messages. The inline comment appropriately documents this design decision.openspec/changes/archive/2026-01-12-add-mcp-server/specs/cli-init/spec.md (1)
1-29: LGTM!The delta spec correctly follows the coding guidelines:
- Uses
## MODIFIED Requirementsfor the behavior change (directory location change fromopenspec/to.openspec/)- Uses
## ADDED Requirementsfor the new orthogonal Legacy Migration capability- Both requirements include properly formatted
#### Scenario:headers- MODIFIED section includes the full requirement block with header and scenario as required
src/mcp/prompts.ts (1)
1-50: LGTM! Clean MCP prompt registration.The implementation follows a consistent pattern across all three prompts, properly imports shared templates from the core module, and correctly uses the FastMCP
addPromptAPI. The template composition with newline separators is appropriate for readability in the prompt output.test/core/init.test.ts (1)
69-69: LGTM! Path updates align with the.openspecdirectory migration.The test expectations have been correctly updated to use the new hidden directory structure (
.openspec/instead ofopenspec/). This is consistent with the changes insrc/core/config.tswhereDEFAULT_OPENSPEC_DIR_NAME = ".openspec"and the migration logic insrc/core/init.ts.Also applies to: 87-87, 810-810, 819-819, 830-830, 850-850
openspec/specs/cli-init/spec.md (2)
22-22: LGTM! Directory naming updated to.openspec/.The spec correctly reflects the migration to a hidden directory structure, reducing project root clutter while maintaining the same internal organization.
Also applies to: 28-28
312-329: LGTM! New safety and migration requirements follow spec conventions.The new "Slash Command Safety" and "Legacy Migration" requirements appropriately use SHALL for normative language and include well-structured scenarios with GIVEN/WHEN/THEN format. These additions align with the broader directory migration work in this PR.
src/mcp/server.ts (1)
16-24: LGTM!The server start flow correctly registers tools, resources, and prompts before starting the stdio transport. The
awaiton sync registration functions is harmless and provides forward-compatibility if those functions become async later.src/core/update.ts (2)
3-3: LGTM!The migration to
resolveOpenSpecDircorrectly enables dynamic directory resolution, supporting both legacyopenspec/and new.openspec/directories while maintaining backward compatibility.Also applies to: 11-11
90-91: LGTM!Using
path.basename(openspecPath)correctly extracts the actual directory name (eitheropenspecor.openspec) for accurate instruction file path construction.openspec/changes/agent-only-mcp-workflow/specs/mcp-server/spec.md (1)
1-34: LGTM!The spec delta file follows the required format: uses
## ADDED Requirementsheader for new capabilities, each requirement includes at least one#### Scenario:with the correct 4-hashtag format. The requirements appropriately describe orthogonal MCP server capabilities. Based on coding guidelines.src/core/config.ts (1)
1-3: LGTM!The three-constant approach cleanly separates concerns:
DEFAULT_OPENSPEC_DIR_NAMEfor new projects,LEGACY_OPENSPEC_DIR_NAMEfor backward compatibility checks, andOPENSPEC_DIR_NAMEas the canonical alias. This enables smooth migration fromopenspec/to.openspec/.src/core/init.ts (1)
13-13: LGTM on new imports.The imports for
confirmfrom@inquirer/promptsand the directory name constants are correctly added to support the migration flow.Also applies to: 24-25
src/mcp/tools.ts (5)
1-10: LGTM on imports and module structure.The imports are well-organized, bringing in the necessary dependencies for MCP tool registration including FastMCP, Zod for schemas, and the core OpenSpec commands/utilities.
33-50: LGTM on list_specs tool.The implementation correctly handles errors and returns structured JSON output.
52-100: LGTM on show_change and show_spec tools.Both tools properly handle the format parameter (json/markdown) and delegate to the respective command classes with appropriate error handling.
102-125: LGTM on validate_change tool.The tool correctly resolves the OpenSpec directory, constructs the change path, and uses the Validator with optional strict mode.
127-155: Archive tool correctly usesyes: trueto avoid interactive prompts.Good practice for MCP context where interactive prompts aren't supported. The tool properly surfaces both skipSpecs and noValidate options.
src/commands/change.ts (3)
9-9: LGTM on path-resolver import.Correctly imports the new
resolveOpenSpecDirfunction for dynamic path resolution.
30-38: LGTM on getChangeMarkdown method.Clean implementation with proper error handling that provides a user-friendly message including the expected path.
72-72: LGTM on dynamic path resolution updates.The methods
show,list, andvalidatenow correctly useresolveOpenSpecDirfor dynamic directory resolution, supporting both legacy and default OpenSpec directories.Also applies to: 100-104, 114-114, 202-202
src/commands/spec.ts (2)
61-68: LGTM on getSpecMarkdown method.Clean implementation with proper error handling and path resolution.
108-113: LGTM on dynamic path resolution updates.The
show,list, andvalidatecommand actions now correctly useresolveOpenSpecDirfor consistent path resolution across all spec operations.Also applies to: 150-160, 229-230
src/core/list.ts (3)
7-8: LGTM on new imports.The imports for
resolveOpenSpecDirandFileSystemUtilsare correctly added to support the new modular functions.
10-20: LGTM on new exported interfaces.The
ChangeInfoandSpecInfointerfaces provide clear type definitions for the data returned by the new functions, enabling better type safety for consumers.
158-214: LGTM on ListCommand refactor.The
ListCommand.executemethod now properly delegates to the newlistChangesandlistSpecsfunctions, maintaining the same output formatting while benefiting from the modular data-gathering logic.
| ## 3. Verification | ||
| - [ ] 3.1 Verify `openspec_init` works via an MCP client (e.g., Gemini CLI) in a fresh directory. | ||
| - [ ] 3.2 Verify `openspec_update` refreshes files correctly. | ||
| - [ ] 3.3 Verify `openspec_create_change` scaffolds a new change directory. | ||
| - [ ] 3.4 Ensure the CLI remains functional for users who prefer it. |
There was a problem hiding this comment.
Duplicate section number: "3. Verification" should be "5. Verification".
There are two sections numbered "3": "3. Build & CI Validation" (line 16) and "3. Verification" (line 25). This should be section 5 to maintain sequential numbering. The task IDs (3.1, 3.2, etc.) should also be updated to 5.1, 5.2, etc.
Proposed fix
-## 3. Verification
-- [ ] 3.1 Verify `openspec_init` works via an MCP client (e.g., Gemini CLI) in a fresh directory.
-- [ ] 3.2 Verify `openspec_update` refreshes files correctly.
-- [ ] 3.3 Verify `openspec_create_change` scaffolds a new change directory.
-- [ ] 3.4 Ensure the CLI remains functional for users who prefer it.
+## 5. Verification
+- [ ] 5.1 Verify `openspec_init` works via an MCP client (e.g., Gemini CLI) in a fresh directory.
+- [ ] 5.2 Verify `openspec_update` refreshes files correctly.
+- [ ] 5.3 Verify `openspec_create_change` scaffolds a new change directory.
+- [ ] 5.4 Ensure the CLI remains functional for users who prefer it.Based on learnings, tasks.md should contain implementation checklist with numbered sections and checkbox items.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 3. Verification | |
| - [ ] 3.1 Verify `openspec_init` works via an MCP client (e.g., Gemini CLI) in a fresh directory. | |
| - [ ] 3.2 Verify `openspec_update` refreshes files correctly. | |
| - [ ] 3.3 Verify `openspec_create_change` scaffolds a new change directory. | |
| - [ ] 3.4 Ensure the CLI remains functional for users who prefer it. | |
| ## 5. Verification | |
| - [ ] 5.1 Verify `openspec_init` works via an MCP client (e.g., Gemini CLI) in a fresh directory. | |
| - [ ] 5.2 Verify `openspec_update` refreshes files correctly. | |
| - [ ] 5.3 Verify `openspec_create_change` scaffolds a new change directory. | |
| - [ ] 5.4 Ensure the CLI remains functional for users who prefer it. |
🤖 Prompt for AI Agents
In @openspec/changes/agent-only-mcp-workflow/tasks.md around lines 25 - 29,
Rename the duplicate "3. Verification" header to "5. Verification" and update
its checklist item IDs from 3.1–3.4 to 5.1–5.4 so numbering is sequential with
the earlier "3. Build & CI Validation"; specifically modify the header text "3.
Verification" and the four task prefixes (e.g., change "3.1 Verify
`openspec_init`..." to "5.1 Verify `openspec_init`...", and likewise for
3.2→5.2, 3.3→5.3, 3.4→5.4) to preserve a consistent numbered checklist structure
in tasks.md.
openspec/changes/archive/2026-01-12-add-mcp-server/specs/mcp-server/spec.md
Show resolved
Hide resolved
| ## 7. Verification | ||
| - [x] 6.1 Verify `openspec serve` starts and communicates over stdio. | ||
| - [x] 6.2 Verify tools, resources, and prompts are discoverable by an MCP client. | ||
| - [x] 6.3 Verify `openspec init` creates `.openspec/`. |
There was a problem hiding this comment.
Section numbering inconsistency.
Section 7 "Verification" contains items numbered 6.1, 6.2, 6.3 instead of 7.1, 7.2, 7.3.
📝 Proposed fix
## 7. Verification
-- [x] 6.1 Verify `openspec serve` starts and communicates over stdio.
-- [x] 6.2 Verify tools, resources, and prompts are discoverable by an MCP client.
-- [x] 6.3 Verify `openspec init` creates `.openspec/`.
+- [x] 7.1 Verify `openspec serve` starts and communicates over stdio.
+- [x] 7.2 Verify tools, resources, and prompts are discoverable by an MCP client.
+- [x] 7.3 Verify `openspec init` creates `.openspec/`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 7. Verification | |
| - [x] 6.1 Verify `openspec serve` starts and communicates over stdio. | |
| - [x] 6.2 Verify tools, resources, and prompts are discoverable by an MCP client. | |
| - [x] 6.3 Verify `openspec init` creates `.openspec/`. | |
| ## 7. Verification | |
| - [x] 7.1 Verify `openspec serve` starts and communicates over stdio. | |
| - [x] 7.2 Verify tools, resources, and prompts are discoverable by an MCP client. | |
| - [x] 7.3 Verify `openspec init` creates `.openspec/`. |
🤖 Prompt for AI Agents
In @openspec/changes/archive/2026-01-12-add-mcp-server/tasks.md around lines 32
- 35, Under the "## 7. Verification" header update the three checklist item
prefixes from 6.x to 7.x (change 6.1/6.2/6.3 to 7.1/7.2/7.3) so the subsection
numbering matches the section heading; locate the checklist lines under the
"Verification" heading in tasks.md and adjust only the numeric prefixes while
keeping the rest of each line unchanged.
| export async function listChanges(targetPath: string, sort: 'recent' | 'name' = 'recent'): Promise<ChangeInfo[]> { | ||
| const openspecPath = await resolveOpenSpecDir(targetPath); | ||
| const changesDir = path.join(openspecPath, 'changes'); | ||
|
|
||
| // Check if changes directory exists | ||
| try { | ||
| await fs.access(changesDir); | ||
| } catch { | ||
| throw new Error("No OpenSpec changes directory found. Run 'openspec init' first."); | ||
| } | ||
| // Check if changes directory exists | ||
| if (!await FileSystemUtils.directoryExists(changesDir)) { | ||
| // Return empty if directory doesn't exist, or throw? The original code threw error. | ||
| throw new Error("No OpenSpec changes directory found. Run 'openspec init' first."); | ||
| } | ||
|
|
||
| // Get all directories in changes (excluding archive) | ||
| const entries = await fs.readdir(changesDir, { withFileTypes: true }); | ||
| const changeDirs = entries | ||
| .filter(entry => entry.isDirectory() && entry.name !== 'archive') | ||
| .map(entry => entry.name); | ||
| // Get all directories in changes (excluding archive) | ||
| const entries = await fs.readdir(changesDir, { withFileTypes: true }); | ||
| const changeDirs = entries | ||
| .filter(entry => entry.isDirectory() && entry.name !== 'archive') | ||
| .map(entry => entry.name); | ||
|
|
||
| if (changeDirs.length === 0) { | ||
| if (json) { | ||
| console.log(JSON.stringify({ changes: [] })); | ||
| } else { | ||
| console.log('No active changes found.'); | ||
| } | ||
| return; | ||
| } | ||
| if (changeDirs.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| // Collect information about each change | ||
| const changes: ChangeInfo[] = []; | ||
| // Collect information about each change | ||
| const changes: ChangeInfo[] = []; | ||
|
|
||
| for (const changeDir of changeDirs) { | ||
| for (const changeDir of changeDirs) { | ||
| const progress = await getTaskProgressForChange(changesDir, changeDir); | ||
| const changePath = path.join(changesDir, changeDir); | ||
| const lastModified = await getLastModified(changePath); | ||
| changes.push({ | ||
| name: changeDir, | ||
| completedTasks: progress.completed, | ||
| totalTasks: progress.total, | ||
| lastModified | ||
| name: changeDir, | ||
| completedTasks: progress.completed, | ||
| totalTasks: progress.total, | ||
| lastModified | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| // Sort by preference (default: recent first) | ||
| if (sort === 'recent') { | ||
| // Sort by preference (default: recent first) | ||
| if (sort === 'recent') { | ||
| changes.sort((a, b) => b.lastModified.getTime() - a.lastModified.getTime()); | ||
| } else { | ||
| } else { | ||
| changes.sort((a, b) => a.name.localeCompare(b.name)); | ||
| } | ||
|
|
||
| return changes; | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling between listChanges and listSpecs.
listChanges throws an error when the changes directory doesn't exist (line 91), while listSpecs returns an empty array (line 134). This inconsistency could confuse consumers. Consider aligning the behavior—either both throw or both return empty arrays.
🔧 Option: Return empty array for consistency
// Check if changes directory exists
if (!await FileSystemUtils.directoryExists(changesDir)) {
- // Return empty if directory doesn't exist, or throw? The original code threw error.
- throw new Error("No OpenSpec changes directory found. Run 'openspec init' first.");
+ return [];
}Then handle the "not initialized" message at the ListCommand level if needed.
🤖 Prompt for AI Agents
In @src/core/list.ts around lines 84 - 127, listChanges currently throws when
the changes directory is missing while listSpecs returns [], causing
inconsistent behavior; modify listChanges (the function listChanges and its use
of FileSystemUtils.directoryExists) to return an empty array when the changesDir
does not exist (replace the throw new Error(...) with `return []`), keep the
rest of the function intact, and ensure any callers (e.g., ListCommand) handle
an empty result for "not initialized" messaging instead of relying on an
exception.
| export function registerTools(server: FastMCP) { | ||
| server.addTool({ | ||
| name: "openspec_list_changes", | ||
| description: "List active OpenSpec changes.", | ||
| parameters: z.object({ | ||
| sort: z.enum(['recent', 'name']).optional().default('recent'), | ||
| }), | ||
| execute: async (args) => { | ||
| try { | ||
| const changes = await listChanges(process.cwd(), args.sort); | ||
| return { | ||
| content: [{ type: "text", text: JSON.stringify(changes, null, 2) }] | ||
| }; | ||
| } catch (error: any) { | ||
| return { | ||
| isError: true, | ||
| content: [{ type: "text", text: `Error listing changes: ${error.message}` }] | ||
| }; | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how process.cwd() is used in src/mcp/
rg -n "process\.cwd\(\)" src/mcp/ -C2Repository: Fission-AI/OpenSpec
Length of output: 2361
🏁 Script executed:
#!/bin/bash
# List all files in src/mcp/ to understand the structure
fd . src/mcp/ --type fRepository: Fission-AI/OpenSpec
Length of output: 154
🏁 Script executed:
#!/bin/bash
# Search for FastMCP server initialization and configuration
rg -n "FastMCP\|registerTools\|new.*MCP" src/ -A3 -B1 | head -100Repository: Fission-AI/OpenSpec
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check server initialization and FastMCP setup
cat -n src/mcp/server.tsRepository: Fission-AI/OpenSpec
Length of output: 841
🏁 Script executed:
#!/bin/bash
# Check index.ts for server setup
cat -n src/mcp/index.tsRepository: Fission-AI/OpenSpec
Length of output: 395
🏁 Script executed:
#!/bin/bash
# Find the resolveOpenSpecDir function to understand working directory handling
rg -n "resolveOpenSpecDir" src/ -B3 -A10Repository: Fission-AI/OpenSpec
Length of output: 18997
Add project path parameter support for MCP tools and resources to enable operation from arbitrary working directories.
All MCP tools and resources rely on process.cwd() to determine the project root. This creates a critical issue: if the MCP server is started from a different directory than the project being worked on, all operations fail. The underlying functions (listChanges, listSpecs, resolveOpenSpecDir) already support a projectRoot parameter, but the MCP layer doesn't expose this to clients.
Add a projectPath parameter to tool parameters (optional, defaulting to process.cwd() for backward compatibility) and pass it through to the core functions. Alternatively, support project path configuration at server startup via OpenSpecMCPServer constructor or environment variable, then make it available to resource loaders.
This affects all tools in src/mcp/tools.ts and all resource loaders in src/mcp/resources.ts.
🤖 Prompt for AI Agents
In @src/mcp/tools.ts around lines 11 - 31, The MCP tools use process.cwd() as
the project root causing failures when the server runs outside the project;
update registerTools (and all other tool registrations in src/mcp/tools.ts) to
add an optional "projectPath" parameter (defaulting to process.cwd()) to each
tool's z.object parameters, and pass that value into core functions (e.g., call
listChanges(projectPath, args.sort) instead of listChanges(process.cwd(), ...));
alternatively, expose a server-level projectRoot via the OpenSpecMCPServer
constructor and use that as the default when args.projectPath is not provided —
make the same change for all usages of process.cwd() in tools and for resource
loaders in src/mcp/resources.ts (e.g., resolveOpenSpecDir, listSpecs) so they
accept and use the projectPath/projectRoot parameter.
Refactor core logic into standalone files (init-logic.ts, update-logic.ts, etc.) to enable reuse between CLI and MCP server. Expand OpenSpec MCP server with tools for init, update, view, create_change, and archive_change. Update documentation to reflect the new zero-install workflow and MCP capabilities.
Extract artifact workflow, configuration, and bulk validation logic into pure core functions. Register new MCP tools: openspec_config_*, openspec_artifact_*, openspec_validate_all, and openspec_list_schemas.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/commands/artifact-workflow.ts (2)
110-110: Hardcoded 'openspec' path may not work with new.openspecdirectory.This line uses
path.join(projectRoot, 'openspec', 'changes')but the PR introduces.openspecas the new default directory name. This should useresolveOpenSpecDirfor consistency with the rest of the codebase.🐛 Proposed fix using path resolver
+import { resolveOpenSpecDir } from '../core/path-resolver.js'; + async function validateChangeExists( changeName: string | undefined, projectRoot: string ): Promise<string> { - const changesPath = path.join(projectRoot, 'openspec', 'changes'); + const openspecPath = await resolveOpenSpecDir(projectRoot); + const changesPath = path.join(openspecPath, 'changes');Note: This will require making
validateChangeExistsasync-aware for the path resolution, but it already is async.
503-503: Same hardcoded path issue ingenerateApplyInstructions.Line 503 also uses the hardcoded
'openspec'path. This should be updated to useresolveOpenSpecDirfor consistency.🐛 Proposed fix
async function generateApplyInstructions( projectRoot: string, changeName: string, schemaName?: string ): Promise<ApplyInstructions> { // loadChangeContext will auto-detect schema from metadata if not provided const context = loadChangeContext(projectRoot, changeName, schemaName); - const changeDir = path.join(projectRoot, 'openspec', 'changes', changeName); + const openspecPath = await resolveOpenSpecDir(projectRoot); + const changeDir = path.join(openspecPath, 'changes', changeName);src/core/specs-apply.ts (1)
363-364: UseresolveOpenSpecDirto support bothopenspecand.openspecdirectories.The hardcoded
'openspec'directory names at lines 363-364 break compatibility with the new.openspecdirectory structure. The project explicitly supports both viaresolveOpenSpecDir, which other functions in the codebase (archive-logic.ts, update-logic.ts, etc.) already use. SinceapplySpecsis async, it should resolve the path dynamically:const openspecPath = await resolveOpenSpecDir(projectRoot); const changeDir = path.join(openspecPath, 'changes', changeName); const mainSpecsDir = path.join(openspecPath, 'specs');This ensures the function works when the project migrates to
.openspecas the default.
🤖 Fix all issues with AI agents
In @src/core/archive-logic.ts:
- Around line 48-56: The catch-all around fs.stat in archive-logic.ts masks real
errors; update the try/catch that checks changeDir (using fs.stat and
changeName) to only convert ENOENT/ENOTDIR into the "Change '... not found"
Error and rethrow any other errors (or include the original error) so
permission/I/O failures are preserved; specifically inspect the caught error
(e.g., err.code) and if it's not a not-found code, rethrow or wrap it with the
original error message instead of returning the generic not-found message.
In @src/core/view.ts:
- Around line 68-71: The catch block inside ViewCommand (the try around
getViewData()) calls process.exit(1), which prevents tests from catching errors
and lets library code control process lifecycle; change that catch to re-throw
the caught error (e.g., throw error) so the CLI handler can decide to exit and
tests can assert failures; locate the catch in src/core/view.ts around the
getViewData()/ViewCommand code and replace the process.exit(1) call with
throwing the error.
🧹 Nitpick comments (18)
src/mcp/resources.ts (3)
12-20: Add error handling for file read operations.The
loadfunction doesn't handle file-not-found or read errors gracefully. When a user requests a non-existent change or spec, the rawENOENTerror will propagate to the MCP client.Also, the
@ts-ignoresuggests a type mismatch with FastMCP's expected signature—consider properly typingvariablesor addressing the underlying type issue.♻️ Proposed fix with error handling
// @ts-ignore load: async (variables: any) => { + try { const openspecPath = await resolveOpenSpecDir(process.cwd()); const filePath = path.join(openspecPath, 'changes', variables.name, 'proposal.md'); const text = await fs.readFile(filePath, 'utf-8'); return { content: [{ uri: `openspec://changes/${variables.name}/proposal`, text }] }; + } catch (error: any) { + if (error.code === 'ENOENT') { + throw new Error(`Change '${variables.name}' not found or has no proposal.md`); + } + throw error; + } }
28-36: Same error handling concern for Change Tasks resource.Apply similar error handling as suggested for the proposal resource to provide user-friendly error messages when the tasks file doesn't exist.
44-52: Same error handling concern for Specification resource.The spec resource should also handle missing files gracefully with a descriptive error message.
src/mcp/tools.ts (1)
6-6: Remove unused import.
ArchiveCommandis imported but never used. The code usesrunArchivefromarchive-logic.jsinstead.🧹 Remove unused import
-import { ArchiveCommand } from '../core/archive.js';test/core/archive.test.ts (1)
79-79: Consider escaping variable in regex pattern.The static analysis flagged potential ReDoS risk at lines 79, 339, 375, 414 where
changeNameis interpolated into a regex. While the risk is minimal in tests (controlled input), escaping the variable is a good defensive practice.🔒 Defensive regex escaping
- expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`)); + // Escape special regex characters in changeName for safety + const escapedName = changeName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${escapedName}`));Alternatively, since the date format and name are predictable in tests, you could use string matching:
expect(archives[0]).toContain(changeName); expect(archives[0]).toMatch(/^\d{4}-\d{2}-\d{2}-/);src/mcp/server.ts (1)
1-29: Clean MCP server implementation that correctly follows FastMCP patterns.The use of
createRequirefor ESM-compatible JSON imports is appropriate, and the registration of tools, resources, and prompts before starting the server is properly sequenced. All required modules are correctly exported and the server is appropriately configured for stdio transport.Consider adding error handling around
start()to provide better error messages if the transport fails to initialize, though FastMCP may already handle this internally.src/core/archive.ts (1)
99-109: Dead code: validation failure check is unreachable.Since
runOptionsalways setsthrowOnValidationError: true(line 56), validation failures will throwValidationErrorand be caught in the try-catch block (lines 78-93). ThevalidationReport.validcheck here will never befalsebecauserunArchiveeither throws or returns early with an emptyarchiveNamewhen validation fails without throwing.Consider removing this redundant block or making the behavior consistent.
Suggested fix
- if (result.validationReport && !result.validationReport.valid) { - console.log(chalk.red(`\nValidation failed for '${changeName}':`)); - for (const issue of result.validationReport.issues) { - if (issue.level === 'ERROR') { - console.log(chalk.red(` ✗ ${issue.message}`)); - } else if (issue.level === 'WARNING') { - console.log(chalk.yellow(` ⚠ ${issue.message}`)); - } - } - return; - }src/core/init-logic.ts (2)
89-96: Simplify duplicate branching logic.Both branches call the same functions with only
skipExistingdiffering based onextendMode.Suggested simplification
- // Step 1: Create directory structure - if (!extendMode) { - await createDirectoryStructure(openspecPath); - await writeTemplateFiles(openspecPath, { aiTools: selectedToolIds }, false); - } else { - await createDirectoryStructure(openspecPath); - await writeTemplateFiles(openspecPath, { aiTools: selectedToolIds }, true); - } + // Step 1: Create directory structure + await createDirectoryStructure(openspecPath); + await writeTemplateFiles(openspecPath, { aiTools: selectedToolIds }, extendMode);
134-179: Code duplication:isToolConfiguredexists in bothinit-logic.tsandinit.ts.The
isToolConfiguredfunction at lines 134-179 is nearly identical to the one insrc/core/init.ts(lines 472-517). This violates DRY and creates maintenance burden.Consider exporting this function from
init-logic.tsand importing it ininit.ts, or moving it to a shared utility module.src/core/archive-logic.ts (2)
14-22: Consider stronger typing forvalidationReport.Using
anyforvalidationReport(line 18) and theValidationError.report(line 25) loses type safety. If aValidationReporttype exists insrc/core/validation/validator.ts, consider using it here.Also applies to: 24-29
101-109: Avoid creating Validator instance inside loop.A new
Validator()is instantiated for each spec in the loop (line 104). This is inefficient ifValidatorhas any initialization overhead.Suggested fix
+ const specValidator = new Validator(); for (const p of prepared) { const specName = path.basename(path.dirname(p.update.target)); if (!skipValidation) { - const report = await new Validator().validateSpecContent(specName, p.rebuilt); + const report = await specValidator.validateSpecContent(specName, p.rebuilt); if (!report.valid) { if (options.throwOnValidationError) throw new ValidationError(report); throw new Error(`Validation failed for rebuilt spec: ${specName}`); } }src/core/init.ts (1)
454-517: Code duplication withinit-logic.ts.As noted in the
init-logic.tsreview,getExistingToolStatesandisToolConfiguredare duplicated here. Consider consolidating to a single shared implementation.src/core/view-logic.ts (4)
1-1: Consider using asyncfs/promisesconsistently.The file imports synchronous
fsmethods but the codebase (e.g.,src/core/specs-apply.ts) usesimport { promises as fs } from 'fs'for async operations. Using sync methods likeexistsSync,readdirSync, andreadFileSynccan block the event loop, especially when scanning many directories.♻️ Suggested import change
-import * as fs from 'fs'; +import { promises as fs } from 'fs'; +import { existsSync } from 'fs';Then update the internal calls to use
await fs.readdir(),await fs.readFile(), etc., keeping onlyexistsSyncfor quick existence checks where needed.
43-43: SyncreaddirSyncblocks the event loop.This function is marked
asyncbut uses synchronousreaddirSync. For consistency with the rest of the codebase and to avoid blocking on larger directories, consider usingawait fs.readdir()fromfs/promises.♻️ Proposed fix
- const entries = fs.readdirSync(changesDir, { withFileTypes: true }); + const entries = await fs.readdir(changesDir, { withFileTypes: true });
82-98: Spec directories withoutspec.mdare silently excluded.When a directory exists under
specs/but contains nospec.mdfile (line 86 check), it's silently skipped. This might hide misconfigured specs. Consider either:
- Including them with
requirementCount: 0(like the error case on line 94), or- Logging a warning for visibility.
Also, line 88 uses sync
readFileSyncinside anasyncfunction—consider usingawait fs.readFile()for consistency.♻️ Proposed fix for async consistency
if (fs.existsSync(specFile)) { try { - const content = fs.readFileSync(specFile, 'utf-8'); + const content = await fs.readFile(specFile, 'utf-8'); const parser = new MarkdownParser(content); const spec = parser.parseSpec(entry.name); const requirementCount = spec.requirements.length; specs.push({ name: entry.name, requirementCount }); } catch (error) { specs.push({ name: entry.name, requirementCount: 0 }); } + } else { + // Spec directory exists but has no spec.md - include with 0 requirements + specs.push({ name: entry.name, requirementCount: 0 }); }
99-100: Specs are not sorted before returning.The
getChangesDatafunction sorts all three arrays (draft,active,completed) before returning, butgetSpecsDatareturns specs unsorted. Per the AI summary, specs should have "deterministic ordering." Consider sorting by name for consistency.♻️ Proposed fix
+ specs.sort((a, b) => a.name.localeCompare(b.name)); + return specs; }src/core/view.ts (1)
6-71: Inconsistent indentation within the try block.The code inside the
tryblock uses 8-space indentation (lines 7-67) while the rest of the class uses 4-space indentation. This appears to be an extra level of indentation.♻️ Suggested fix (showing first few lines)
try { - const data = await getViewData(targetPath); - - console.log(chalk.bold('\nOpenSpec Dashboard\n')); - console.log('═'.repeat(60)); + const data = await getViewData(targetPath); + + console.log(chalk.bold('\nOpenSpec Dashboard\n')); + console.log('═'.repeat(60));Apply similar 4-space indentation throughout the try block.
src/core/specs-apply.ts (1)
439-441: Minor: Inconsistent indentation.Lines 439-441 use extra indentation compared to surrounding code.
♻️ Proposed fix
totals.removed += p.counts.removed; totals.renamed += p.counts.renamed; - if (p.ignoredRemovals > 0) { - ignoredRemovals.push({ specName: capability, count: p.ignoredRemovals }); - } + if (p.ignoredRemovals > 0) { + ignoredRemovals.push({ specName: capability, count: p.ignoredRemovals }); + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.github/workflows/ci.yml.github/workflows/release-prepare.ymlGEMINI.mdREADME.mdgemini-extension.jsonpackage.jsonscripts/sync-extension-version.mjssrc/commands/artifact-workflow.tssrc/core/archive-logic.tssrc/core/archive.tssrc/core/change-logic.tssrc/core/init-logic.tssrc/core/init.tssrc/core/specs-apply.tssrc/core/templates/agents-template.tssrc/core/update-logic.tssrc/core/update.tssrc/core/view-logic.tssrc/core/view.tssrc/mcp/prompts.tssrc/mcp/resources.tssrc/mcp/server.tssrc/mcp/tools.tstest/core/archive.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- GEMINI.md
- src/mcp/prompts.ts
- gemini-extension.json
- package.json
🧰 Additional context used
🧠 Learnings (14)
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Use `openspec archive <change-id> --skip-specs --yes` for tooling-only changes
Applied to files:
test/core/archive.test.tssrc/core/archive-logic.tssrc/core/archive.tssrc/core/templates/agents-template.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Check `openspec/project.md` for project conventions before creating specs
Applied to files:
test/core/archive.test.tsREADME.mdsrc/core/templates/agents-template.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
test/core/archive.test.tssrc/core/change-logic.tsREADME.mdsrc/core/templates/agents-template.tssrc/core/specs-apply.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Move completed changes from `changes/[name]/` to `changes/archive/YYYY-MM-DD-[name]/` after deployment
Applied to files:
test/core/archive.test.tssrc/core/archive.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
test/core/archive.test.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval
Applied to files:
test/core/archive.test.ts.github/workflows/ci.ymlsrc/core/archive.tssrc/core/templates/agents-template.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
test/core/archive.test.tssrc/core/specs-apply.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
test/core/archive.test.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
test/core/archive.test.tsREADME.mdsrc/core/specs-apply.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
test/core/archive.test.ts
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
README.mdsrc/core/templates/agents-template.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
README.mdsrc/core/templates/agents-template.ts
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Always open `@/openspec/AGENTS.md` when the request mentions planning or proposals (words like proposal, spec, change, plan), introduces new capabilities, breaking changes, architecture shifts, or performance/security work, or sounds ambiguous and needs the authoritative spec before coding
Applied to files:
README.mdsrc/core/templates/agents-template.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Search existing work using `openspec spec list --long`, `openspec list` for enumerating changes; use `rg` only for full-text search
Applied to files:
README.mdsrc/core/templates/agents-template.ts
🧬 Code graph analysis (13)
src/mcp/server.ts (5)
build.js (1)
require(7-7)scripts/check-extension-version.mjs (1)
pkg(3-3)src/mcp/tools.ts (1)
registerTools(16-239)src/mcp/resources.ts (1)
registerResources(6-54)src/mcp/prompts.ts (1)
registerPrompts(20-62)
src/mcp/tools.ts (9)
src/core/init-logic.ts (1)
runInit(30-118)src/core/update-logic.ts (1)
runUpdate(18-99)src/core/view-logic.ts (1)
getViewData(16-30)src/core/change-logic.ts (1)
runCreateChange(15-48)src/core/list.ts (2)
listChanges(84-127)listSpecs(129-156)src/commands/spec.ts (1)
SpecCommand(60-115)src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/core/validation/validator.ts (1)
Validator(16-449)src/core/archive-logic.ts (1)
runArchive(31-145)
src/core/init-logic.ts (5)
src/core/config.ts (5)
LEGACY_OPENSPEC_DIR_NAME(2-2)DEFAULT_OPENSPEC_DIR_NAME(1-1)AI_TOOLS(21-43)OPENSPEC_MARKERS(5-8)OpenSpecConfig(10-12)src/utils/file-system.ts (1)
FileSystemUtils(44-213)src/core/configurators/registry.ts (1)
ToolRegistry(11-49)src/core/configurators/slash/registry.ts (1)
SlashCommandRegistry(23-81)src/core/templates/index.ts (1)
TemplateManager(14-47)
test/core/archive.test.ts (1)
src/core/config.ts (1)
DEFAULT_OPENSPEC_DIR_NAME(1-1)
src/core/update.ts (1)
src/core/update-logic.ts (1)
runUpdate(18-99)
src/core/change-logic.ts (2)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/utils/file-system.ts (1)
FileSystemUtils(44-213)
src/core/update-logic.ts (5)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/utils/file-system.ts (2)
FileSystemUtils(44-213)fileExists(88-98)src/core/templates/agents-template.ts (1)
agentsTemplate(1-465)src/core/configurators/registry.ts (1)
ToolRegistry(11-49)src/core/configurators/slash/registry.ts (1)
SlashCommandRegistry(23-81)
src/core/view-logic.ts (3)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/utils/task-progress.ts (1)
getTaskProgressForChange(27-35)src/core/parsers/markdown-parser.ts (1)
MarkdownParser(10-237)
src/core/archive.ts (4)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/core/specs-apply.ts (1)
findSpecUpdates(56-95)src/core/archive-logic.ts (2)
ArchiveResult(14-22)runArchive(31-145)src/utils/task-progress.ts (1)
formatTaskStatus(37-41)
src/core/view.ts (1)
src/core/view-logic.ts (2)
getViewData(16-30)DashboardData(7-14)
src/core/init.ts (5)
src/core/config.ts (4)
LEGACY_OPENSPEC_DIR_NAME(2-2)DEFAULT_OPENSPEC_DIR_NAME(1-1)AI_TOOLS(21-43)OPENSPEC_MARKERS(5-8)src/core/init-logic.ts (1)
runInit(30-118)src/core/styles/palette.ts (1)
PALETTE(3-8)src/core/configurators/registry.ts (1)
ToolRegistry(11-49)src/core/configurators/slash/registry.ts (1)
SlashCommandRegistry(23-81)
src/mcp/resources.ts (1)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)
src/commands/artifact-workflow.ts (1)
src/core/change-logic.ts (1)
runCreateChange(15-48)
🪛 ast-grep (0.40.4)
test/core/archive.test.ts
[warning] 338-338: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\d{4}-\\d{2}-\\d{2}-${changeName})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 413-413: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\d{4}-\\d{2}-\\d{2}-${changeName})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (34)
.github/workflows/ci.yml (2)
162-164: LGTM! MCP server smoke test.Good addition to verify the
servecommand is properly wired up after build. Running--helpis a lightweight check that ensures the command exists and parses without errors.
148-150: LGTM! Extension version sync check addresses reviewer concerns.This CI step directly addresses concerns about keeping extension and CLI versions synchronized. The check will fail the build if
gemini-extension.jsonversion drifts frompackage.json, ensuring they stay in sync automatically.scripts/sync-extension-version.mjs (1)
1-16: LGTM! Simple and effective version sync script.The script correctly synchronizes versions and the lack of explicit try/catch is acceptable here—uncaught exceptions from missing files or invalid JSON will naturally fail the CI job with a clear stack trace.
.github/workflows/release-prepare.yml (1)
43-43: LGTM! Extension version stays in sync during releases.Using
ci:versionensures thegemini-extension.jsonversion is updated atomically withpackage.jsonduring the changesets version bump. This directly addresses the release coordination concern raised in PR comments.src/core/update-logic.ts (4)
8-16: Well-designed result interface.The
UpdateResultinterface comprehensively tracks all outcomes, enabling both CLI output and MCP tool responses to provide detailed feedback.
41-71: LGTM! Robust per-configurator error handling.The try/catch per configurator allows partial success—a failure in one tool configuration won't prevent others from being updated. Error details are captured for reporting.
73-88: LGTM! Slash command update handling.Correctly checks
isAvailablebefore attempting updates and captures failures without blocking other configurators.
27-29: This review comment is based on a misunderstanding of the file paths. The two writes target different files in different directories:
- Line 29: Writes to
openspecPath/AGENTS.md(the primary template in.openspec/oropenspec/directory)- Line 61: AgentsStandardConfigurator writes to
resolvedProjectPath/AGENTS.md(the root assistant stub in the project root)There is no double-write issue. The flow is intentional: first refresh the main AGENTS.md template in the OpenSpec directory, then update/create the project root's AGENTS.md stub using marker-based updates to preserve manual additions.
src/core/templates/agents-template.ts (1)
143-150: LGTM! Documentation accurately reflects implementation.Clear and concise explanation of CLI vs MCP integration modes. The MCP tool name
openspec_list_changesis correctly implemented and matches the template documentation. This helps users understand the zero-install workflow enabled by the Gemini extension.src/mcp/tools.ts (1)
16-239: LGTM! Comprehensive MCP tools implementation.The tools registration is well-structured with:
- Consistent error handling pattern across all tools
- Proper use of Zod for parameter validation
- Clear tool descriptions for MCP discoverability
- Appropriate use of existing core logic modules
The coverage of init, update, view, CRUD operations for changes/specs, validation, and archiving provides a complete MCP interface for OpenSpec workflows.
test/core/archive.test.ts (2)
4-4: Good use of configuration constant.Importing
DEFAULT_OPENSPEC_DIR_NAMEfrom config and using it throughout the tests ensures the tests stay in sync with production path resolution logic.
382-415: Test refactored correctly for interactive mode with noValidate.The mock confirm sequence and path updates correctly test the interactive decline scenario with the new directory structure.
src/core/change-logic.ts (2)
15-48: Clean implementation of change creation logic.The function follows a clear pattern: validate inputs → resolve paths → guard against duplicates → create resources → write metadata → return result. Good separation of concerns by delegating to existing utilities.
37-41: No action needed —writeChangeMetadatais synchronous.
writeChangeMetadatais a synchronous function with avoidreturn type; it does not perform async operations. The call does not requireawait.Likely an incorrect or invalid review comment.
README.md (2)
84-103: Clear documentation of integration modes.The new section effectively communicates the two integration paths (Native MCP vs CLI Wrapper) and provides a ready-to-use MCP configuration snippet. This aligns well with the new MCP server implementation.
152-166: Helpful Gemini extension documentation.The installation instructions are clear, and the benefits section effectively communicates the value proposition. The note about still needing the CLI installed globally is an important clarification that prevents user confusion.
src/commands/artifact-workflow.ts (2)
30-31: Good refactor to use centralized change creation logic.The switch from
createChangetorunCreateChangeproperly leverages the newchange-logic.tsmodule, ensuring consistent change creation across CLI and MCP interfaces.
759-770: LGTM! Correct usage of runCreateChange result.The refactored code properly uses
result.changeDirandresult.schemafrom therunCreateChangereturn value, which provides the actual paths created rather than constructing them separately.src/core/archive.ts (3)
5-7: LGTM!Clean refactor to use centralized path resolution and archive logic. The imports align with the new modular architecture.
55-73: Good UX: Pre-archive confirmation for spec updates.Checking for spec updates before invoking
runArchiveand prompting the user allows for early abort without side effects.
95-97: Inconsistent error handling: throwing after try-catch.When
result.alreadyExistsis true, anErroris thrown. However, this occurs outside the try-catch block, resulting in an unhandled exception that will propagate to the caller instead of being handled gracefully like other errors.Consider logging and returning instead of throwing, for consistency with other error paths.
Suggested fix
if (result.alreadyExists) { - throw new Error(`Archive '${result.archiveName}' already exists.`); + console.log(chalk.red(`Archive '${result.archiveName}' already exists.`)); + return; }Likely an incorrect or invalid review comment.
src/core/update.ts (3)
3-9: LGTM!Clean delegation to centralized
runUpdatelogic. The destructuring provides clear access to all result fields.
11-19: LGTM!Good error handling pattern—logging individual failures with clear distinction between slash commands and regular files.
21-57: LGTM!Summary construction is well-structured with cross-platform path normalization for slash commands.
src/core/init-logic.ts (3)
14-28: LGTM!Well-defined types for initialization results. The
InitResultinterface provides comprehensive information about the initialization outcome.
44-55: Clarify migration precedence when both directories exist.When both legacy (
openspec/) and default (.openspec/) directories exist (line 52), the code falls through to use the legacy path. This means an existing.openspec/directory is ignored in favor ofopenspec/.Is this the intended behavior? Consider adding a comment explaining this precedence or logging a warning when both exist.
181-249: LGTM!Helper functions are well-structured with clear single responsibilities. Directory structure creation, template writing, and tool configuration are cleanly separated.
src/core/archive-logic.ts (1)
31-145: LGTM overall!The
runArchivefunction provides a clean, centralized archive workflow with proper validation, spec updates, and result aggregation. The separation from the CLI command layer enables reuse in MCP and other contexts.src/core/init.ts (3)
386-400: LGTM!Good user experience: prompting for migration when legacy directory exists provides a clear upgrade path.
402-425: Minor redundancy:extendModecomputed twice.
extendModeis computed here (line 405) for the UI prompt, butrunInitalso computes it internally. This is acceptable for separation of concerns (UI vs. logic), but the duplication is worth noting.The commented code (lines 428-431) correctly explains the spinner is for UX—consider condensing these comments.
667-848: LGTM!The success message display and remaining UI logic are well-structured with clear formatting and helpful user guidance.
src/core/view.ts (1)
2-2: LGTM!Good refactor to separate data gathering (
view-logic.ts) from rendering (view.ts). The import ofDashboardDataprovides proper type safety for the display methods.src/core/specs-apply.ts (2)
46-46: LGTM!Good addition of
ignoredRemovalsto the output interface. This provides transparency about REMOVED operations that were ignored when applying deltas to new specs, improving debuggability.
204-218: LGTM on the ignoredRemovals tracking for new specs.Clean handling of the edge case where REMOVED operations target a non-existent spec. Counting them as
ignoredRemovalsrather than silently skipping or erroring is a reasonable approach that provides feedback to callers.
| // Verify change exists | ||
| try { | ||
| const stat = await fs.stat(changeDir); | ||
| if (!stat.isDirectory()) { | ||
| throw new Error(`Change '${changeName}' not found.`); | ||
| } | ||
| } catch { | ||
| throw new Error(`Change '${changeName}' not found.`); | ||
| } |
There was a problem hiding this comment.
Catch-all masks underlying errors.
The catch block at line 54 swallows all exceptions (permission errors, I/O failures, etc.) and converts them to a generic "not found" message. This could hide real issues.
Suggested fix
// Verify change exists
try {
const stat = await fs.stat(changeDir);
if (!stat.isDirectory()) {
throw new Error(`Change '${changeName}' not found.`);
}
- } catch {
- throw new Error(`Change '${changeName}' not found.`);
+ } catch (error: any) {
+ if (error.code === 'ENOENT') {
+ throw new Error(`Change '${changeName}' not found.`);
+ }
+ throw error;
}🤖 Prompt for AI Agents
In @src/core/archive-logic.ts around lines 48 - 56, The catch-all around fs.stat
in archive-logic.ts masks real errors; update the try/catch that checks
changeDir (using fs.stat and changeName) to only convert ENOENT/ENOTDIR into the
"Change '... not found" Error and rethrow any other errors (or include the
original error) so permission/I/O failures are preserved; specifically inspect
the caught error (e.g., err.code) and if it's not a not-found code, rethrow or
wrap it with the original error message instead of returning the generic
not-found message.
| } catch (error: any) { | ||
| console.error(chalk.red(error.message)); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ViewCommand usage to understand how it's instantiated and called
rg -n "ViewCommand" --type=ts -B 2 -A 2Repository: Fission-AI/OpenSpec
Length of output: 1891
🏁 Script executed:
#!/bin/bash
# Read the error handling in view.ts around lines 68-71
echo "=== src/core/view.ts ==="
sed -n '60,80p' src/core/view.ts
echo -e "\n=== src/cli/index.ts CLI handler ==="
sed -n '130,145p' src/cli/index.ts
echo -e "\n=== Check test expectations ==="
sed -n '40,60p' test/core/view.test.ts
sed -n '100,120p' test/core/view.test.tsRepository: Fission-AI/OpenSpec
Length of output: 2784
🏁 Script executed:
#!/bin/bash
# Check the full test file to see if process.exit is mocked
cat -n test/core/view.test.ts | head -100Repository: Fission-AI/OpenSpec
Length of output: 4657
🏁 Script executed:
#!/bin/bash
# Search for process.exit mocking or error test cases in the test file
rg -n "process.exit|mock|stub|spy" test/core/view.test.ts
# Also check if there are any error test cases
rg -n "error|Error|throw|catches" test/core/view.test.tsRepository: Fission-AI/OpenSpec
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Let's look at the full view.ts to understand the error flow and what could throw
wc -l src/core/view.tsRepository: Fission-AI/OpenSpec
Length of output: 83
🏁 Script executed:
#!/bin/bash
# View the entire view.ts file to understand error scenarios
cat -n src/core/view.tsRepository: Fission-AI/OpenSpec
Length of output: 5449
Throw the error instead of calling process.exit(1); let the CLI handler manage process termination.
Calling process.exit(1) directly in ViewCommand prevents proper error testing—the tests would terminate the test process if getViewData() threw an error. Additionally, this violates separation of concerns: library code shouldn't control process lifecycle. The CLI handler at src/cli/index.ts:137 already has a try-catch block ready to handle errors and call process.exit(1). Change the catch block to throw the error instead:
} catch (error: any) {
throw error;
}
This allows the CLI to handle termination while keeping ViewCommand testable and reusable in non-CLI contexts (e.g., MCP tools).
🤖 Prompt for AI Agents
In @src/core/view.ts around lines 68 - 71, The catch block inside ViewCommand
(the try around getViewData()) calls process.exit(1), which prevents tests from
catching errors and lets library code control process lifecycle; change that
catch to re-throw the caught error (e.g., throw error) so the CLI handler can
decide to exit and tests can assert failures; locate the catch in
src/core/view.ts around the getViewData()/ViewCommand code and replace the
process.exit(1) call with throwing the error.
…sts plan - Archived the change as it was completed. - Updated proposal and tasks to include unused imports cleanup and emphasize test migration. - Includes file moves related to previous refactoring.
Refactor list/show/validate logic from ChangeCommand to src/core/change-logic.ts. Add comprehensive tests for core change logic and new MCP server tests covering tools, resources, and prompts. Update mcp-server spec with test requirements.
Refactor InitCommand to use dynamic imports for @InQuirer modules. Add mandatory descriptions to @ts-expect-error in MCP resources.
Update openspec_show_change execution to use refactored core change logic (getChangeMarkdown, getChangeJson) instead of invoking ChangeCommand methods.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/validate.ts (1)
200-202: Unused variablemaxSuggestions.The variable
maxSuggestionsis declared but never used in this function.🧹 Suggested fix
const DEFAULT_CONCURRENCY = 6; - const maxSuggestions = 5; // used by nearestMatches const concurrency = normalizeConcurrency(opts.concurrency) ?? normalizeConcurrency(process.env.OPENSPEC_CONCURRENCY) ?? DEFAULT_CONCURRENCY;
🤖 Fix all issues with AI agents
In @openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.md:
- Around line 18-31: Consolidate the three duplicate "## Impact" sections into a
single "## Impact" header by merging all unique bullet points and removing
duplicates (specifically remove the repeated "Architecture Cleanliness" entry),
preserving the points "Architecture Cleanliness", "Full Parity", "Continuous
Reliability", "Flexibility", "Adoption", "Zero-Install Adoption", "Consistent
Agent Experience", and "Future-Proofing" and reordering them for clarity if
needed so there is only one coherent Impact list under a single header.
In @openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md:
- Around line 25-29: The markdown has duplicate "## 3." headings; update the
second heading ("## 3. Verification") to "## 5. Verification" so section
numbering is sequential; locate the heading text in tasks.md and change the
header line that reads "## 3. Verification" to "## 5. Verification" and verify
any internal links or references to "Verification" still work after renaming.
In @src/commands/archive.ts:
- Around line 95-97: The code currently performs expensive validation/build
steps inside runArchive before returning alreadyExists, wasting work; update
runArchive in archive-logic.ts to perform an early existence check and return {
alreadyExists: true, archiveName: ... } immediately if found (so callers like
the check in archive.ts can fail fast), or if deferred checking is intentional,
add a clear comment in runArchive and in archive.ts explaining that
validation/build must run prior to existence checking and why; locate the logic
in runArchive and move the archive existence lookup to the top (before the
validation/build blocks) or add the documentation explaining the design choice.
In @src/core/config-logic.ts:
- Around line 43-55: The code mutates the in-memory config after validation and
may leave it inconsistent if saveGlobalConfig fails; instead, apply the
validated newConfig when persisting or revert on error. Change the flow in the
block around getGlobalConfig/coerceValue/setNestedValue/validateConfig so you
either call saveGlobalConfig(newConfig as GlobalConfig) and only replace the
live config (e.g., setNestedValue(config, key, coercedValue) or assign returned
saved config) after save succeeds, or wrap the current mutation +
saveGlobalConfig(config) in a try-catch that rolls back the in-memory change on
error; ensure validateConfig still runs against newConfig and preserve
references to newConfig, config, setNestedValue, validateConfig, and
saveGlobalConfig when making the update.
In @src/core/validation-logic.ts:
- Around line 43-60: The loops that build changeDir and file currently hardcode
'openspec' and should use resolveOpenSpecDir() from path-resolver.ts; call
resolveOpenSpecDir() once (e.g., const openSpecDir = resolveOpenSpecDir()) and
replace path.join(process.cwd(), 'openspec', ...) with path.join(openSpecDir,
'changes', id) for validateChangeDeltaSpecs and path.join(openSpecDir, 'specs',
id, 'spec.md') for validateSpec so the validator.validateChangeDeltaSpecs and
validator.validateSpec calls work for both 'openspec/' and '.openspec/' layouts.
🧹 Nitpick comments (15)
src/core/validation-logic.ts (2)
29-118: Significant code duplication withsrc/commands/validate.ts.The bulk validation logic here (queue building, concurrency control, result aggregation) and the helper functions (
summarizeType,normalizeConcurrency,getPlannedId,getPlannedType) are nearly identical to the implementation insrc/commands/validate.ts(lines 193-335). This violates DRY.Consider having
validate.tsimport and userunBulkValidationfrom this core module, adding only the UI layer (spinner, colored output) on top.
38-39: Consider adding an upper bound on concurrency.While the default of 6 is reasonable, there's no maximum limit. A very high value from
OPENSPEC_CONCURRENCYcould exhaust file handles. Consider capping at a reasonable maximum (e.g., 32).💡 Suggested improvement
+ const MAX_CONCURRENCY = 32; const DEFAULT_CONCURRENCY = 6; - const concurrency = normalizeConcurrency(opts.concurrency) ?? normalizeConcurrency(process.env.OPENSPEC_CONCURRENCY) ?? DEFAULT_CONCURRENCY; + const concurrency = Math.min( + normalizeConcurrency(opts.concurrency) ?? normalizeConcurrency(process.env.OPENSPEC_CONCURRENCY) ?? DEFAULT_CONCURRENCY, + MAX_CONCURRENCY + );src/core/config-logic.ts (1)
63-72: Consider validating config afterunsetConfigValue.Unlike
setConfigValue, this function doesn't validate the config after deletion. Deleting certain keys could potentially leave the config in an invalid state. Consider adding post-deletion validation for consistency.💡 Suggested improvement
export function unsetConfigValue(key: string): boolean { const config = getGlobalConfig() as Record<string, unknown>; const existed = deleteNestedValue(config, key); if (existed) { + const validation = validateConfig(config); + if (!validation.success) { + throw new Error(`Unsetting "${key}" would leave config invalid: ${validation.error}`); + } saveGlobalConfig(config as GlobalConfig); } return existed; }test/core/view.test.ts (1)
23-34: Inconsistent OpenSpec directory naming across tests.This test uses
openspec(line 24) whiletest/core/update.test.tsuses.openspec(line 25). Per the AI summary, the PR migrates fromopenspec/to.openspec/. Consider using.openspecconsistently across all tests to match the new convention.♻️ Suggested fix
it('should return empty dashboard data for new project', async () => { - const openspecPath = path.join(tempDir, 'openspec'); + const openspecPath = path.join(tempDir, '.openspec'); await fs.mkdir(openspecPath, { recursive: true });test/core/list.test.ts (1)
21-21: Consider removing the nesteddescribe('execute')block.This appears to be a leftover from the command-based testing pattern. Since
listChangesis now a function (not a class with anexecutemethod), the nested describe block adds unnecessary nesting.test/core/update.test.ts (1)
1-6: Unused import:vifrom vitest.The
vimock utility is imported but not used in any test. Consider removing it to keep imports clean.♻️ Suggested fix
-import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest';src/commands/list.ts (2)
65-75: Potential edge case:Math.maxwith empty spread returns-Infinity.Although the
changes.length === 0check on line 43 guards against reaching this code with an empty array, if the control flow changes in the future,Math.max(...[])would return-Infinity, causingpadEnd(-Infinity)to behave unexpectedly.Consider adding a fallback for defensive coding:
🔧 Suggested defensive fix
// Display results console.log('Changes:'); const padding = ' '; - const nameWidth = Math.max(...changes.map(c => c.name.length)); + const nameWidth = changes.length > 0 ? Math.max(...changes.map(c => c.name.length)) : 0; for (const change of changes) {
85-91: SameMath.maxedge case for specs mode.The specs mode has the same potential issue. For consistency, consider applying the same defensive pattern here.
🔧 Suggested defensive fix
console.log('Specs:'); const padding = ' '; - const nameWidth = Math.max(...specs.map(s => s.id.length)); + const nameWidth = specs.length > 0 ? Math.max(...specs.map(s => s.id.length)) : 0; for (const spec of specs) {src/core/list.ts (3)
1-8: Redundant imports can be consolidated.The file imports
path(line 2) and thenjoinfrompath(line 5). Similarly, it importspromises as fsfromfs(line 1) andreadFileSyncfromfs(line 4). Consider consolidating these imports for clarity.Suggested consolidation
-import { promises as fs } from 'fs'; -import path from 'path'; +import { promises as fs, readFileSync } from 'fs'; +import path, { join } from 'path'; import { getTaskProgressForChange } from '../utils/task-progress.js'; -import { readFileSync } from 'fs'; -import { join } from 'path'; import { MarkdownParser } from './parsers/markdown-parser.js';
17-20: Minor: Inconsistent indentation in SpecInfo interface.The
SpecInfointerface uses 4-space indentation whileChangeInfouses 2-space indentation. Consider aligning for consistency.Suggested fix
-export interface SpecInfo { - id: string; - requirementCount: number; +export interface SpecInfo { + id: string; + requirementCount: number; }
100-127: Inconsistent async pattern: using synchronousreadFileSyncin an async function.
listSpecsis an async function but uses synchronousreadFileSync(line 115), which blocks the event loop. For consistency with the rest of the codebase and better performance when listing many specs, consider usingfs.promises.readFile.Additionally,
listSpecsreturns an empty array when the specs directory is missing, whilelistChangesthrows an error for a missing changes directory. Consider whether this inconsistency is intentional.Suggested fix for async consistency
for (const id of specDirs) { const specPath = join(specsDir, id, 'spec.md'); try { - const content = readFileSync(specPath, 'utf-8'); + const content = await fs.readFile(specPath, 'utf-8'); const parser = new MarkdownParser(content); const spec = parser.parseSpec(id); specs.push({ id, requirementCount: spec.requirements.length }); } catch { // If spec cannot be read or parsed, include with 0 count specs.push({ id, requirementCount: 0 }); } }src/core/artifact-logic.ts (3)
56-58: Mixed sync/async filesystem operations.The function uses
fs.existsSyncandfs.statSync(line 57) while elsewhere usingfs.promises.readdir. For consistency and to avoid blocking the event loop, consider usingfs.promises.statwith a try/catch pattern.Suggested async alternative
- const exists = fs.existsSync(changePath) && fs.statSync(changePath).isDirectory(); + let exists = false; + try { + const stat = await fs.promises.stat(changePath); + exists = stat.isDirectory(); + } catch { + exists = false; + }
268-273: Clarify contextFiles content: paths vs file contents.The
contextFilesproperty inApplyInstructionsis typed asRecord<string, string>. The current implementation stores file paths (line 271), but the naming and type could suggest it stores file contents. Consider either renaming tocontextFilePathsor adding a clarifying comment.
346-348: Dynamic import could be a static import.The
getUserSchemasDirfunction is imported dynamically on line 346. Unless there's a specific reason for lazy loading (e.g., circular dependency avoidance), consider using a static import at the top of the file for consistency and slightly better performance.Suggested change
Add to imports at top of file:
import { getUserSchemasDir } from './artifact-graph/resolver.js';Then simplify the function:
- const { getUserSchemasDir } = await import('./artifact-graph/resolver.js'); - const userSchemasDir = getUserSchemasDir(); + const userSchemasDir = getUserSchemasDir();src/commands/init.ts (1)
427-435: Consider: Tool spinner may be misleading.The comment on lines 428-431 acknowledges that
runInitalready configured the tools, and the spinner is just for CLI experience. While this works, it could be misleading ifrunInitfails during tool configuration—the spinner would never start. Consider either removing the redundant spinner or havingrunInitaccept a progress callback for more accurate feedback.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
openspec/changes/add-mcp-tests/.openspec.yamlopenspec/changes/add-mcp-tests/proposal.mdopenspec/changes/add-mcp-tests/specs/mcp-server/spec.mdopenspec/changes/add-mcp-tests/tasks.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/proposal.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/tasks.mdopenspec/specs/cli-validate/spec.mdopenspec/specs/mcp-server/spec.mdsrc/cli/index.tssrc/commands/archive.tssrc/commands/init.tssrc/commands/list.tssrc/commands/update.tssrc/commands/validate.tssrc/commands/view.tssrc/core/artifact-logic.tssrc/core/config-logic.tssrc/core/list.tssrc/core/validation-logic.tssrc/mcp/tools.tstest/core/archive.test.tstest/core/init.test.tstest/core/list.test.tstest/core/update.test.tstest/core/view.test.ts
✅ Files skipped from review due to trivial changes (1)
- openspec/changes/add-mcp-tests/.openspec.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- openspec/specs/mcp-server/spec.md
🧰 Additional context used
📓 Path-based instructions (5)
openspec/changes/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Scaffold proposal using
proposal.md,tasks.md, optionaldesign.md, and delta specs underopenspec/changes/<id>/
Files:
openspec/changes/add-mcp-tests/proposal.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.mdopenspec/changes/add-mcp-tests/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/tasks.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdopenspec/changes/add-mcp-tests/tasks.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/proposal.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md
openspec/changes/*/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
proposal.mdincludes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Files:
openspec/changes/add-mcp-tests/proposal.md
openspec/changes/**/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/**/specs/**/spec.md: Use## ADDED|MODIFIED|REMOVED|RENAMED Requirementsheaders in spec delta files
Include at least one#### Scenario:per requirement in spec delta files
Use#### Scenario: Nameformat (4 hashtags) for scenario headers, not bullets or bold text
Use## ADDED Requirementsfor new orthogonal capabilities that can stand alone; use## MODIFIED Requirementsfor behavior changes of existing requirements
When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Files:
openspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.mdopenspec/changes/add-mcp-tests/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.md
openspec/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Files:
openspec/specs/cli-validate/spec.md
openspec/changes/*/tasks.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
tasks.mdcontains implementation checklist with numbered sections and checkbox items
Files:
openspec/changes/add-mcp-tests/tasks.md
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
openspec/changes/add-mcp-tests/proposal.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.mdopenspec/changes/add-mcp-tests/specs/mcp-server/spec.mdopenspec/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdopenspec/changes/add-mcp-tests/tasks.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/proposal.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.mdsrc/commands/init.tsopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
openspec/changes/add-mcp-tests/proposal.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdopenspec/changes/add-mcp-tests/tasks.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.mdsrc/commands/init.tsopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
openspec/changes/add-mcp-tests/proposal.mdopenspec/changes/add-mcp-tests/specs/mcp-server/spec.mdsrc/core/list.tssrc/core/artifact-logic.tsopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdopenspec/changes/add-mcp-tests/tasks.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
openspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.mdopenspec/changes/add-mcp-tests/specs/mcp-server/spec.mdopenspec/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdtest/core/archive.test.tsopenspec/changes/add-mcp-tests/tasks.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
openspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.mdopenspec/changes/add-mcp-tests/specs/mcp-server/spec.mdopenspec/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdtest/core/archive.test.tsopenspec/changes/add-mcp-tests/tasks.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
openspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.mdopenspec/changes/add-mcp-tests/specs/mcp-server/spec.mdopenspec/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdtest/core/archive.test.tsopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
openspec/changes/add-mcp-tests/specs/mcp-server/spec.mdopenspec/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
test/core/list.test.tsopenspec/changes/archive/2026-01-12-refactor-core-isolation/tasks.mdopenspec/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdopenspec/changes/add-mcp-tests/tasks.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Search existing work using `openspec spec list --long`, `openspec list` for enumerating changes; use `rg` only for full-text search
Applied to files:
src/core/list.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval
Applied to files:
openspec/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.mdsrc/core/validation-logic.tssrc/commands/validate.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/specs/**/spec.md : Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Applied to files:
openspec/specs/cli-validate/spec.mdopenspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `#### Scenario: Name` format (4 hashtags) for scenario headers, not bullets or bold text
Applied to files:
openspec/specs/cli-validate/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Move completed changes from `changes/[name]/` to `changes/archive/YYYY-MM-DD-[name]/` after deployment
Applied to files:
src/commands/archive.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Use `openspec archive <change-id> --skip-specs --yes` for tooling-only changes
Applied to files:
src/commands/archive.tstest/core/archive.test.ts
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Always open `@/openspec/AGENTS.md` when the request mentions planning or proposals (words like proposal, spec, change, plan), introduces new capabilities, breaking changes, architecture shifts, or performance/security work, or sounds ambiguous and needs the authoritative spec before coding
Applied to files:
openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Check `openspec/project.md` for project conventions before creating specs
Applied to files:
openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Create `design.md` only when needed: cross-cutting changes, new external dependencies, significant data model changes, security/performance complexity, or pre-coding ambiguity
Applied to files:
openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.mdopenspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md
🧬 Code graph analysis (10)
src/core/config-logic.ts (1)
src/core/config-schema.ts (7)
getNestedValue(64-79)validateConfigKeyPath(31-55)coerceValue(142-162)setNestedValue(89-103)validateConfig(218-230)deleteNestedValue(112-130)DEFAULT_CONFIG(21-23)
test/core/list.test.ts (1)
src/core/list.ts (1)
listChanges(55-98)
src/commands/list.ts (2)
src/core/list.ts (2)
listChanges(55-98)listSpecs(100-127)src/utils/task-progress.ts (1)
formatTaskStatus(37-41)
src/commands/archive.ts (3)
src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/utils/task-progress.ts (2)
getTaskProgressForChange(27-35)formatTaskStatus(37-41)src/core/archive.ts (1)
ArchiveCommand(13-302)
src/commands/view.ts (2)
src/core/view-logic.ts (2)
getViewData(16-30)DashboardData(7-14)src/core/view.ts (1)
ViewCommand(7-219)
src/cli/index.ts (1)
src/commands/serve.ts (1)
ServeCommand(3-8)
src/core/validation-logic.ts (2)
src/utils/item-discovery.ts (1)
getActiveChangeIds(4-23)src/core/validation/validator.ts (1)
Validator(16-449)
test/core/view.test.ts (1)
src/core/view-logic.ts (1)
getViewData(16-30)
test/core/update.test.ts (1)
src/core/update-logic.ts (1)
runUpdate(18-99)
src/commands/init.ts (5)
src/core/config.ts (3)
LEGACY_OPENSPEC_DIR_NAME(2-2)DEFAULT_OPENSPEC_DIR_NAME(1-1)AI_TOOLS(21-43)src/utils/file-system.ts (1)
FileSystemUtils(44-213)src/core/styles/palette.ts (1)
PALETTE(3-8)src/core/configurators/registry.ts (1)
ToolRegistry(11-49)src/core/configurators/slash/registry.ts (1)
SlashCommandRegistry(23-81)
🪛 LanguageTool
openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.md
[style] ~20-~20: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...Full Parity*: Ensures agents have the exact same "superpowers" as users on the command l...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md
[uncategorized] ~18-~18: The official name of this software platform is spelled with a capital “H”.
Context: ...CLI after refactoring. - [x] 3.2 Update .github/workflows/ci.yml to include a check th...
(GITHUB)
🔇 Additional comments (63)
openspec/specs/cli-validate/spec.md (1)
219-227: LGTM! Clear success guidance for validation workflow.The new requirement properly specifies post-validation guidance using correct normative language (SHALL) and follows the established format with descriptive text and a well-structured scenario.
openspec/changes/archive/2026-01-12-refactor-core-isolation/tasks.md (1)
1-34: LGTM! Tasks document follows the required format.The document properly uses numbered sections with checkbox items as required by project conventions. All tasks are marked complete, appropriate for an archived change.
src/core/validation-logic.ts (1)
68-101: LGTM!The promise-based concurrency pool implementation is well-structured with proper error handling. The catch block correctly captures failed tasks and records them as results with error-level issues.
src/core/config-logic.ts (1)
17-28: LGTM!Clean wrapper functions that provide a consistent API layer over the lower-level config operations.
src/commands/validate.ts (1)
159-166: LGTM!The "Next steps" guidance for successful change validation is well-implemented with clear, actionable suggestions. This aligns with the "Success Guidance" requirement in the cli-validate spec delta.
openspec/changes/archive/2026-01-12-refactor-core-isolation/specs/cli-validate/spec.md (1)
1-10: LGTM!The spec delta follows the required conventions:
## ADDED Requirementsheader, normativeSHALLlanguage, and proper#### Scenario:format with WHEN/THEN/AND structure. As per coding guidelines, this is correctly structured.openspec/changes/archive/2026-01-12-refactor-core-isolation/specs/mcp-server/spec.md (1)
1-9: LGTM!The spec delta correctly documents the shared core logic requirement with proper formatting:
## ADDED Requirementsheader, normativeSHALLlanguage, and well-structured scenario. This aligns with the PR's goal of ensuring MCP and CLI share the same core business logic.openspec/changes/add-mcp-tests/specs/mcp-server/spec.md (1)
1-26: LGTM!The spec delta correctly documents MCP server testing requirements:
- Proper
## ADDED Requirementsheader- Normative
SHALLlanguage throughout- Multiple well-structured
#### Scenario:blocks- Clear separation between "Test Coverage" (verifying exposed interfaces) and "Testability of Core Logic" (ensuring pure function design)
This supports the PR's goal of ensuring core logic is testable independently of transport layers.
openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.md (1)
3-17: LGTM!The "Why" and "What Changes" sections follow the expected proposal structure with clear motivation and well-organized change list.
openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/specs/mcp-server/spec.md (1)
1-34: LGTM!This spec delta file follows all coding guidelines perfectly:
- Uses proper "## ADDED Requirements" header for new orthogonal capabilities
- Each requirement includes at least one scenario with the correct "#### Scenario:" format
- Scenarios follow clear WHEN/THEN structure
- Requirements are well-defined with clear SHALL statements
openspec/changes/add-mcp-tests/tasks.md (1)
1-29: LGTM!This tasks file follows all coding guidelines:
- Contains implementation checklist with numbered sections
- All tasks use proper checkbox format
- Well-organized with clear categories (Spec Updates, Refactoring, Testing, Cleanup, Verification)
- Tasks are specific and actionable
src/commands/update.ts (3)
1-9: LGTM!The imports and delegation to
runUpdateare clean. Destructuring the result provides clear access to all relevant fields for building the summary.
11-19: LGTM!The error logging correctly distinguishes between slash tool errors and regular file errors using the
slash:prefix convention.
21-57: LGTM!The summary building logic is well-structured:
- Properly separates instruction files from AI tool files.
- Uses POSIX path normalization for cross-platform log consistency.
- Consolidates failure reporting from multiple sources.
test/core/view.test.ts (3)
5-7: LGTM!The test suite correctly targets the new
getViewDatafunction, aligning with the PR's shift from command classes to pure data APIs.
19-21: LGTM!The test correctly verifies the error handling when the OpenSpec directory is not initialized.
36-58: LGTM!The test correctly verifies change categorization across draft, active, and completed states. The test data setup and assertions are well-structured.
Note: Same directory naming inconsistency as flagged above (
openspecvs.openspec).openspec/changes/archive/2026-01-12-refactor-core-isolation/proposal.md (1)
1-27: LGTM!The proposal follows the required structure with Why, What Changes, and Impact sections. The documentation clearly explains the architectural separation between core logic and CLI presentation, supporting reusability and testability goals.
Based on learnings, the proposal includes the required sections. Since this is archived and represents completed work with no external breaking changes, the format is appropriate.
test/core/list.test.ts (2)
5-7: LGTM!The test suite correctly targets the new
listChangesfunction, aligning with the migration to pure data APIs.
22-120: LGTM!Comprehensive test coverage for
listChanges:
- Error handling for missing directory
- Empty state handling
- Archive exclusion
- Task counting and completion detection
- Sorting behavior
- Multiple changes with various states
Note: Same
openspecvs.openspecdirectory naming inconsistency as inview.test.ts.test/core/update.test.ts (2)
8-18: LGTM!Good use of
randomUUID()for unique test directory names, preventing potential collisions in parallel test execution.
20-33: LGTM!The tests correctly verify:
- Error handling when OpenSpec is not initialized
- Successful
AGENTS.mdupdate with content verificationThis test uses
.openspec(the new convention), whileview.test.tsandlist.test.tsuseopenspec. Consider aligning all tests to use.openspecfor consistency.src/commands/list.ts (2)
1-7: LGTM!Clean imports and well-defined
ListOptionsinterface. The separation of presentation logic from core data fetching aligns well with the architectural goals of this PR.
9-34: Good placement of presentation logic.The comment on lines 10-14 is helpful. Moving
formatRelativeTimehere is the right call since it's purely for console output formatting. Consider extracting to a sharedutils/format.tsif this pattern is needed elsewhere.src/commands/view.ts (3)
1-72: LGTM! Clean dashboard implementation.The
executemethod properly:
- Fetches data from the core layer via
getViewData- Renders distinct sections for draft, active, completed changes, and specs
- Handles errors gracefully with colored output
The data-driven approach aligns well with the PR's architectural goals.
74-108: Well-structured summary display.The
displaySummarymethod correctly aggregates task progress across active changes and conditionally displays draft count only when non-zero. Good attention to detail.
110-121: Progress bar handles edge cases correctly.The
total === 0check on line 111 prevents division by zero and provides a sensible fallback display. Clean implementation.src/mcp/tools.ts (5)
1-17: LGTM! Clean imports and well-organized tool registration.Good separation of concerns with imports from core logic modules (
init-logic,update-logic,view-logic, etc.) rather than CLI commands where appropriate.
18-39: LGTM! Init tool properly configured.The
openspec_inittool correctly exposes thetoolsandshouldMigrateparameters matching the corerunInitAPI.
266-329: LGTM! Config tools follow consistent patterns.The
openspec_config_get,openspec_config_set, andopenspec_config_listtools properly expose configuration management with appropriate parameters.
331-416: LGTM! Artifact workflow tools properly registered.The artifact-related tools (
openspec_artifact_status,openspec_artifact_instructions,openspec_apply_instructions,openspec_list_schemas) provide comprehensive artifact workflow support for MCP clients.
216-238: Concurrency parameter type is intentional and correct.The
concurrencyparameter is correctly defined asz.string(). The functionrunBulkValidationaccepts a string and internally callsnormalizeConcurrency(defined at src/core/validation-logic.ts:128-133), which parses it to a number usingparseInt. This is a standard pattern for API tool parameters that come as strings and are converted internally. No change needed.Likely an incorrect or invalid review comment.
src/cli/index.ts (3)
7-18: LGTM! Import paths updated correctly.The refactoring from
../core/...to../commands/...forUpdateCommand,ListCommand,ArchiveCommand, andViewCommandaligns with the architectural separation of presentation logic from core business logic.
95-108: LGTM! Serve command properly integrated.Good choice using
console.errorfor MCP server errors (line 105) to avoid contaminating stdout, which is critical for stdio-based MCP communication.
68-68: Dynamic import path updated correctly.The
InitCommanddynamic import path change from../core/init.jsto../commands/init.jsis consistent with the refactoring pattern.src/commands/archive.ts (3)
1-8: LGTM! Clean imports.Good use of
resolveOpenSpecDirfrom the centralized path resolver, supporting both legacyopenspec/and new.openspec/directory structures.
27-53: Good UX: Pre-archive confirmations.The flow properly handles:
- Validation skip warning with user confirmation
- Incomplete tasks warning with option to proceed
This matches the expected interactive CLI experience.
126-169: LGTM! Well-implemented interactive change selection.The
selectChangemethod provides good UX by:
- Displaying task progress inline with change names
- Handling empty changes gracefully
- Gracefully catching Ctrl+C cancellation
The fallback to simple names (lines 154-156) on error is a sensible resilience pattern.
test/core/archive.test.ts (4)
1-6: LGTM! Test imports updated correctly.The test now targets
runArchivefromarchive-logic.js, aligning with the refactored core API.
24-27: LGTM! Error case properly tested.Good use of regex matcher for the error message, allowing flexibility in exact wording while ensuring the key information is present.
29-45: LGTM! Archive success path well tested.The test verifies:
- Correct
changeNamein result- Archive naming pattern (
YYYY-MM-DD-name)- Physical file presence in archive directory
14-17: Test should use.openspec/path for consistency with new conventions.The test creates
openspec/structure (legacy path), while other similar tests likeinit.test.tsandupdate.test.tsuse.openspec/(the default convention). AlthoughresolveOpenSpecDirsupports the legacy path via fallback, this test lacks an explicit comment indicating whether it's intentionally testing legacy support or simply using the outdated path.Update the test to use
.openspec/unless this is specifically meant to verify backward compatibility with legacy directories, in which case add a comment clarifying the intent.⛔ Skipped due to learnings
Learnt from: CR Repo: Fission-AI/OpenSpec PR: 0 File: openspec/AGENTS.md:0-0 Timestamp: 2025-11-25T01:08:19.004Z Learning: Check `openspec/project.md` for project conventions before creating specsLearnt from: CR Repo: Fission-AI/OpenSpec PR: 0 File: openspec/AGENTS.md:0-0 Timestamp: 2025-11-25T01:08:19.004Z Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`test/core/init.test.ts (4)
1-6: LGTM! Test imports updated correctly.The test now targets
runInitfrominit-logic.js, aligning with the refactored core API.
19-31: LGTM! Basic initialization test.Good coverage of the core init behavior:
- Verifies
projectPath,openspecDir, andextendModein result- Confirms physical directory structure (
.openspec/,specs/,changes/,project.md)
33-39: LGTM! Extend mode test.Properly tests that when
.openspec/already exists,extendModeis set totrue.
41-50: LGTM! Migration test covers legacy path handling.Good test ensuring that legacy
openspec/directories are properly migrated to.openspec/whenshouldMigrate: trueis passed. This is important for backward compatibility with existing projects.src/core/list.ts (2)
26-53: LGTM!The recursive
getLastModifiedhelper correctly walks the directory tree to find the most recent file modification time, with a sensible fallback to the directory's own mtime when no files are found.
55-98: LGTM!The
listChangesfunction correctly resolves the OpenSpec directory, validates existence, filters out archived changes, and returns structured data sorted by the specified criteria. The error handling for missing directories is appropriate.src/core/artifact-logic.ts (8)
1-18: LGTM!Imports are well-organized and appropriate for the module's functionality. The use of a default schema constant is a good practice for consistency.
74-83: LGTM!The
validateSchemaExistsfunction provides clear error messages listing available schemas when validation fails.
89-102: LGTM!The
getArtifactStatusfunction correctly validates inputs before loading context and formatting the status response.
108-139: LGTM!The
getArtifactInstructionsfunction has thorough validation with helpful error messages that list valid artifact IDs when the provided one is invalid.
145-165: LGTM!The
TaskItemandApplyInstructionsinterfaces are well-designed with clear typing. The state union type provides good type safety for the workflow states.
167-187: LGTM!The
parseTasksFilefunction correctly parses markdown checkbox syntax, handling both-and*list markers and case-insensitive completion markers.
189-237: LGTM!The
artifactOutputExistsfunction handles both exact paths and glob patterns with good cross-platform path normalization. The recursivehasMatchingFileshelper correctly supports**patterns for deep directory matching.
367-369: LGTM!Simple and clean delegation to
listSchemasWithInfo.src/commands/init.ts (7)
13-27: LGTM!The new imports support the legacy directory migration flow, centralized initialization logic, and tool configuration checks.
386-400: LGTM!The legacy directory detection and migration prompt logic is well-implemented. The confirmation dialog provides clear context and defaults to
true, which is appropriate for encouraging users to adopt the new directory structure.
402-410: LGTM!The
openspecPathdetermination correctly handles all migration scenarios, andextendModeis properly computed based on the target directory's existence.
454-470: LGTM!The
getExistingToolStatesmethod efficiently checks tool configuration states in parallel usingPromise.all.
472-517: LGTM!The
isToolConfiguredmethod correctly handles the variety of tool configuration requirements—some tools need only config files, some only slash commands, and some need both. The marker-based detection is a reliable approach.
628-637: LGTM!The conditional spacer insertion is correctly implemented using the spread operator with a ternary expression.
807-810: LGTM!The
formatToolNamesmethod correctly formats tool names with appropriate styling and comma/conjunction joining.
| # Proposal: Add MCP Server Test Coverage & Core Refactoring | ||
|
|
||
| ## Goal | ||
| Add comprehensive test coverage for the MCP server and refactor CLI logic into Core to enable shared testing. | ||
|
|
||
| ## Motivation | ||
| The MCP server currently lacks dedicated unit and integration tests. Furthermore, significant logic for `change` operations (list, show, validate) resides in `src/commands`, making it difficult to test independently or reuse in the MCP server. | ||
|
|
||
| To ensure reliability and consistency between CLI and MCP, we need to: | ||
| 1. Refactor `list`, `show`, and `validate` logic from `src/commands/change.ts` into `src/core`. | ||
| 2. Add a robust test suite covering Core, MCP, and ensuring CLI integrations work. | ||
|
|
||
| ## Success Criteria | ||
| ### Refactoring | ||
| - [ ] `ChangeCommand` logic in `src/commands/change.ts` refactored into pure functions in `src/core/change-logic.ts` (or similar). | ||
| - [ ] CLI command updated to consume new core functions. | ||
| - [ ] MCP server updated to consume new core functions (if not already). | ||
|
|
||
| ### Testing | ||
| - [ ] **Core**: Unit tests for new `src/core` functions (create, list, show, validate). | ||
| - [ ] **MCP**: Unit tests for `src/mcp/tools.ts`, `resources.ts`, `prompts.ts`. | ||
| - [ ] **MCP**: Integration tests for `src/mcp/server.ts`. | ||
| - [ ] **CLI**: Existing E2E tests pass or are updated to reflect refactoring. | ||
| - [ ] `mcp-server` spec updated to include these requirements. | ||
|
|
||
| ### Cleanup | ||
| - [ ] Remove unused imports across the codebase. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Restructure proposal to use required section headers.
The proposal uses non-standard section headers. As per coding guidelines, proposal.md must include:
## Why(1-2 sentences) — currently labeled "Goal" and "Motivation"## What Changes(bullet list with breaking change markers if applicable) — missing## Impact(affected specs and code) — missing
The "Success Criteria" section does not replace these required sections.
📝 Suggested restructure to align with project conventions
-# Proposal: Add MCP Server Test Coverage & Core Refactoring
-
-## Goal
-Add comprehensive test coverage for the MCP server and refactor CLI logic into Core to enable shared testing.
-
-## Motivation
-The MCP server currently lacks dedicated unit and integration tests. Furthermore, significant logic for `change` operations (list, show, validate) resides in `src/commands`, making it difficult to test independently or reuse in the MCP server.
-
-To ensure reliability and consistency between CLI and MCP, we need to:
-1. Refactor `list`, `show`, and `validate` logic from `src/commands/change.ts` into `src/core`.
-2. Add a robust test suite covering Core, MCP, and ensuring CLI integrations work.
-
-## Success Criteria
-### Refactoring
-- [ ] `ChangeCommand` logic in `src/commands/change.ts` refactored into pure functions in `src/core/change-logic.ts` (or similar).
-- [ ] CLI command updated to consume new core functions.
-- [ ] MCP server updated to consume new core functions (if not already).
-
-### Testing
-- [ ] **Core**: Unit tests for new `src/core` functions (create, list, show, validate).
-- [ ] **MCP**: Unit tests for `src/mcp/tools.ts`, `resources.ts`, `prompts.ts`.
-- [ ] **MCP**: Integration tests for `src/mcp/server.ts`.
-- [ ] **CLI**: Existing E2E tests pass or are updated to reflect refactoring.
-- [ ] `mcp-server` spec updated to include these requirements.
-
-### Cleanup
-- [ ] Remove unused imports across the codebase.
+# Proposal: Add MCP Server Test Coverage & Core Refactoring
+
+## Why
+The MCP server lacks dedicated test coverage, and significant `change` logic resides in CLI commands rather than reusable core functions, hindering independent testing and MCP-CLI consistency.
+
+## What Changes
+- Refactor `list`, `show`, and `validate` logic from `src/commands/change.ts` into `src/core/change-logic.ts`
+- Add comprehensive test suite: Core unit tests, MCP unit/integration tests, CLI E2E validation
+- Update `mcp-server` spec to document testing requirements
+- Remove unused imports across the codebase
+
+## Impact
+**Affected specs:**
+- `openspec/specs/mcp-server/spec.md` (testing requirements)
+
+**Affected code:**
+- `src/commands/change.ts` (refactored to use core)
+- `src/core/change-logic.ts` (new/expanded)
+- `src/mcp/tools.ts`, `src/mcp/resources.ts`, `src/mcp/prompts.ts` (test coverage)
+- `src/mcp/server.ts` (integration tests)
+- New test files under `test/core/`, `test/mcp/`Based on learnings, proposal.md must follow the documented format for consistency across the project.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Proposal: Add MCP Server Test Coverage & Core Refactoring | |
| ## Goal | |
| Add comprehensive test coverage for the MCP server and refactor CLI logic into Core to enable shared testing. | |
| ## Motivation | |
| The MCP server currently lacks dedicated unit and integration tests. Furthermore, significant logic for `change` operations (list, show, validate) resides in `src/commands`, making it difficult to test independently or reuse in the MCP server. | |
| To ensure reliability and consistency between CLI and MCP, we need to: | |
| 1. Refactor `list`, `show`, and `validate` logic from `src/commands/change.ts` into `src/core`. | |
| 2. Add a robust test suite covering Core, MCP, and ensuring CLI integrations work. | |
| ## Success Criteria | |
| ### Refactoring | |
| - [ ] `ChangeCommand` logic in `src/commands/change.ts` refactored into pure functions in `src/core/change-logic.ts` (or similar). | |
| - [ ] CLI command updated to consume new core functions. | |
| - [ ] MCP server updated to consume new core functions (if not already). | |
| ### Testing | |
| - [ ] **Core**: Unit tests for new `src/core` functions (create, list, show, validate). | |
| - [ ] **MCP**: Unit tests for `src/mcp/tools.ts`, `resources.ts`, `prompts.ts`. | |
| - [ ] **MCP**: Integration tests for `src/mcp/server.ts`. | |
| - [ ] **CLI**: Existing E2E tests pass or are updated to reflect refactoring. | |
| - [ ] `mcp-server` spec updated to include these requirements. | |
| ### Cleanup | |
| - [ ] Remove unused imports across the codebase. | |
| # Proposal: Add MCP Server Test Coverage & Core Refactoring | |
| ## Why | |
| The MCP server lacks dedicated test coverage, and significant `change` logic resides in CLI commands rather than reusable core functions, hindering independent testing and MCP-CLI consistency. | |
| ## What Changes | |
| - Refactor `list`, `show`, and `validate` logic from `src/commands/change.ts` into `src/core/change-logic.ts` | |
| - Add comprehensive test suite: Core unit tests, MCP unit/integration tests, CLI E2E validation | |
| - Update `mcp-server` spec to document testing requirements | |
| - Remove unused imports across the codebase | |
| ## Impact | |
| **Affected specs:** | |
| - `openspec/specs/mcp-server/spec.md` (testing requirements) | |
| **Affected code:** | |
| - `src/commands/change.ts` (refactored to use core) | |
| - `src/core/change-logic.ts` (new/expanded) | |
| - `src/mcp/tools.ts`, `src/mcp/resources.ts`, `src/mcp/prompts.ts` (test coverage) | |
| - `src/mcp/server.ts` (integration tests) | |
| - New test files under `test/core/`, `test/mcp/` |
| ## Impact | ||
| - **Architecture Cleanliness**: Enforces separation between presentation (CLI/MCP) and logic (Core). | ||
| - **Full Parity**: Ensures agents have the exact same "superpowers" as users on the command line. | ||
| - **Continuous Reliability**: CI ensures that refactoring for MCP parity never breaks the legacy CLI experience. | ||
|
|
||
| ## Impact | ||
| - **Architecture Cleanliness**: Enforces separation between presentation (CLI/MCP) and logic (Core). | ||
| - **Flexibility**: Users can choose between CLI, MCP, or both. | ||
| - **Adoption**: Significantly lowers the barrier for entry by allowing agents to "self-initialize" via MCP. | ||
|
|
||
| ## Impact | ||
| - **Zero-Install Adoption**: Users only need to add the Gemini extension; no separate CLI installation is required for AI-driven workflows. | ||
| - **Consistent Agent Experience**: Agents interact with a structured API (MCP) rather than parsing CLI output or managing shell command strings. | ||
| - **Future-Proofing**: Aligns OpenSpec with the emerging "plugin" architecture of modern AI coding assistants. |
There was a problem hiding this comment.
Consolidate duplicate Impact sections.
The document contains three separate "## Impact" sections (lines 18, 23, 28), which creates structural confusion and content duplication. The "Architecture Cleanliness" point appears twice (lines 19, 24).
📋 Proposed fix to consolidate Impact sections
## Impact
-- **Architecture Cleanliness**: Enforces separation between presentation (CLI/MCP) and logic (Core).
-- **Full Parity**: Ensures agents have the exact same "superpowers" as users on the command line.
-- **Continuous Reliability**: CI ensures that refactoring for MCP parity never breaks the legacy CLI experience.
-
-## Impact
-- **Architecture Cleanliness**: Enforces separation between presentation (CLI/MCP) and logic (Core).
-- **Flexibility**: Users can choose between CLI, MCP, or both.
-- **Adoption**: Significantly lowers the barrier for entry by allowing agents to "self-initialize" via MCP.
-
-## Impact
+- **Architecture Cleanliness**: Enforces separation between presentation (CLI/MCP) and logic (Core).
+- **Full Parity**: Ensures agents have the same "superpowers" as users on the command line.
+- **Continuous Reliability**: CI ensures that refactoring for MCP parity never breaks the legacy CLI experience.
+- **Flexibility**: Users can choose between CLI, MCP, or both.
- **Zero-Install Adoption**: Users only need to add the Gemini extension; no separate CLI installation is required for AI-driven workflows.
- **Consistent Agent Experience**: Agents interact with a structured API (MCP) rather than parsing CLI output or managing shell command strings.
- **Future-Proofing**: Aligns OpenSpec with the emerging "plugin" architecture of modern AI coding assistants.
+- **Adoption**: Significantly lowers the barrier for entry by allowing agents to "self-initialize" via MCP.🧰 Tools
🪛 LanguageTool
[style] ~20-~20: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...Full Parity*: Ensures agents have the exact same "superpowers" as users on the command l...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
🤖 Prompt for AI Agents
In @openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/proposal.md
around lines 18 - 31, Consolidate the three duplicate "## Impact" sections into
a single "## Impact" header by merging all unique bullet points and removing
duplicates (specifically remove the repeated "Architecture Cleanliness" entry),
preserving the points "Architecture Cleanliness", "Full Parity", "Continuous
Reliability", "Flexibility", "Adoption", "Zero-Install Adoption", "Consistent
Agent Experience", and "Future-Proofing" and reordering them for clarity if
needed so there is only one coherent Impact list under a single header.
| ## 3. Verification | ||
| - [x] 3.1 Verify `openspec_init` works via an MCP client (e.g., Gemini CLI) in a fresh directory. | ||
| - [x] 3.2 Verify `openspec_update` refreshes files correctly. | ||
| - [x] 3.3 Verify `openspec_create_change` scaffolds a new change directory. | ||
| - [x] 3.4 Ensure the CLI remains functional for users who prefer it. |
There was a problem hiding this comment.
Duplicate section number: two "## 3." sections.
There are two sections numbered "3" - "## 3. Build & CI Validation" (line 16) and "## 3. Verification" (line 25). This should be "## 5. Verification" to maintain sequential numbering per the coding guidelines requiring numbered sections.
📝 Suggested fix
-## 3. Verification
+## 5. Verification📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 3. Verification | |
| - [x] 3.1 Verify `openspec_init` works via an MCP client (e.g., Gemini CLI) in a fresh directory. | |
| - [x] 3.2 Verify `openspec_update` refreshes files correctly. | |
| - [x] 3.3 Verify `openspec_create_change` scaffolds a new change directory. | |
| - [x] 3.4 Ensure the CLI remains functional for users who prefer it. | |
| ## 5. Verification | |
| - [x] 3.1 Verify `openspec_init` works via an MCP client (e.g., Gemini CLI) in a fresh directory. | |
| - [x] 3.2 Verify `openspec_update` refreshes files correctly. | |
| - [x] 3.3 Verify `openspec_create_change` scaffolds a new change directory. | |
| - [x] 3.4 Ensure the CLI remains functional for users who prefer it. |
🤖 Prompt for AI Agents
In @openspec/changes/archive/2026-01-12-agent-only-mcp-workflow/tasks.md around
lines 25 - 29, The markdown has duplicate "## 3." headings; update the second
heading ("## 3. Verification") to "## 5. Verification" so section numbering is
sequential; locate the heading text in tasks.md and change the header line that
reads "## 3. Verification" to "## 5. Verification" and verify any internal links
or references to "Verification" still work after renaming.
| if (result.alreadyExists) { | ||
| throw new Error(`Archive '${result.archiveName}' already exists.`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the broader context around lines 95-97 in archive.ts
head -n 120 src/commands/archive.ts | tail -n 35Repository: Fission-AI/OpenSpec
Length of output: 182
🏁 Script executed:
# Now let's find the runArchive function definition
fd -t ts -x grep -l "runArchive" {}Repository: Fission-AI/OpenSpec
Length of output: 232
🏁 Script executed:
# Search for alreadyExists usage across the codebase
rg -n "alreadyExists" --type=tsRepository: Fission-AI/OpenSpec
Length of output: 237
🏁 Script executed:
# Read the archive-logic.ts file to understand the result structure
cat -n src/core/archive-logic.tsRepository: Fission-AI/OpenSpec
Length of output: 5979
🏁 Script executed:
# Read archive.ts around the suspect lines with more context
wc -l src/commands/archive.tsRepository: Fission-AI/OpenSpec
Length of output: 90
🏁 Script executed:
# Get context around lines 95-97 in archive.ts
sed -n '80,110p' src/commands/archive.ts | cat -nRepository: Fission-AI/OpenSpec
Length of output: 1378
Consider checking archive existence before performing expensive operations.
The runArchive function returns successfully with alreadyExists: true (line 130 of archive-logic.ts) rather than throwing, but does so only after validating specs and building updates (lines 61-116). This means all preparation work completes even when the archive already exists, wasting resources.
Either move the archive existence check to the beginning of runArchive to fail fast, or clarify if this deferred check is intentional (e.g., for reporting what would have been archived before the failure).
🤖 Prompt for AI Agents
In @src/commands/archive.ts around lines 95 - 97, The code currently performs
expensive validation/build steps inside runArchive before returning
alreadyExists, wasting work; update runArchive in archive-logic.ts to perform an
early existence check and return { alreadyExists: true, archiveName: ... }
immediately if found (so callers like the check in archive.ts can fail fast), or
if deferred checking is intentional, add a clear comment in runArchive and in
archive.ts explaining that validation/build must run prior to existence checking
and why; locate the logic in runArchive and move the archive existence lookup to
the top (before the validation/build blocks) or add the documentation explaining
the design choice.
| const config = getGlobalConfig() as Record<string, unknown>; | ||
| const coercedValue = coerceValue(value, options.forceString || false); | ||
|
|
||
| const newConfig = JSON.parse(JSON.stringify(config)); | ||
| setNestedValue(newConfig, key, coercedValue); | ||
|
|
||
| const validation = validateConfig(newConfig); | ||
| if (!validation.success) { | ||
| throw new Error(`Invalid configuration - ${validation.error}`); | ||
| } | ||
|
|
||
| setNestedValue(config, key, coercedValue); | ||
| saveGlobalConfig(config as GlobalConfig); |
There was a problem hiding this comment.
Potential state inconsistency if saveGlobalConfig fails.
The code validates newConfig (the clone), then mutates the original config object and calls saveGlobalConfig. If saveGlobalConfig throws, the in-memory config will be out of sync with persisted state.
Consider either:
- Saving
newConfiginstead of mutating the original - Using a try-catch to handle save failures
🔧 Suggested fix
const validation = validateConfig(newConfig);
if (!validation.success) {
throw new Error(`Invalid configuration - ${validation.error}`);
}
- setNestedValue(config, key, coercedValue);
- saveGlobalConfig(config as GlobalConfig);
+ saveGlobalConfig(newConfig as GlobalConfig);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const config = getGlobalConfig() as Record<string, unknown>; | |
| const coercedValue = coerceValue(value, options.forceString || false); | |
| const newConfig = JSON.parse(JSON.stringify(config)); | |
| setNestedValue(newConfig, key, coercedValue); | |
| const validation = validateConfig(newConfig); | |
| if (!validation.success) { | |
| throw new Error(`Invalid configuration - ${validation.error}`); | |
| } | |
| setNestedValue(config, key, coercedValue); | |
| saveGlobalConfig(config as GlobalConfig); | |
| const config = getGlobalConfig() as Record<string, unknown>; | |
| const coercedValue = coerceValue(value, options.forceString || false); | |
| const newConfig = JSON.parse(JSON.stringify(config)); | |
| setNestedValue(newConfig, key, coercedValue); | |
| const validation = validateConfig(newConfig); | |
| if (!validation.success) { | |
| throw new Error(`Invalid configuration - ${validation.error}`); | |
| } | |
| saveGlobalConfig(newConfig as GlobalConfig); |
🤖 Prompt for AI Agents
In @src/core/config-logic.ts around lines 43 - 55, The code mutates the
in-memory config after validation and may leave it inconsistent if
saveGlobalConfig fails; instead, apply the validated newConfig when persisting
or revert on error. Change the flow in the block around
getGlobalConfig/coerceValue/setNestedValue/validateConfig so you either call
saveGlobalConfig(newConfig as GlobalConfig) and only replace the live config
(e.g., setNestedValue(config, key, coercedValue) or assign returned saved
config) after save succeeds, or wrap the current mutation +
saveGlobalConfig(config) in a try-catch that rolls back the in-memory change on
error; ensure validateConfig still runs against newConfig and preserve
references to newConfig, config, setNestedValue, validateConfig, and
saveGlobalConfig when making the update.
| for (const id of changeIds) { | ||
| queue.push(async () => { | ||
| const start = Date.now(); | ||
| const changeDir = path.join(process.cwd(), 'openspec', 'changes', id); | ||
| const report = await validator.validateChangeDeltaSpecs(changeDir); | ||
| const durationMs = Date.now() - start; | ||
| return { id, type: 'change' as const, valid: report.valid, issues: report.issues, durationMs }; | ||
| }); | ||
| } | ||
| for (const id of specIds) { | ||
| queue.push(async () => { | ||
| const start = Date.now(); | ||
| const file = path.join(process.cwd(), 'openspec', 'specs', id, 'spec.md'); | ||
| const report = await validator.validateSpec(file); | ||
| const durationMs = Date.now() - start; | ||
| return { id, type: 'spec' as const, valid: report.valid, issues: report.issues, durationMs }; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for path resolver module and directory conventions
echo "=== Path resolver module ==="
fd -t f 'path-resolver' --exec cat {}
echo ""
echo "=== Directory references in core modules ==="
rg -n "openspec|\.openspec" src/core/ --type ts | head -50
echo ""
echo "=== Check project.md for conventions ==="
if [ -f "openspec/project.md" ]; then
cat openspec/project.md | head -100
fi
echo ""
echo "=== Check if .openspec directory exists ==="
ls -la | grep -E "openspec|\.openspec"
echo ""
echo "=== Search entire codebase for .openspec references ==="
rg "\.openspec" --type ts | head -30
echo ""
echo "=== Check validation-logic.ts lines 40-60 ==="
if [ -f "src/core/validation-logic.ts" ]; then
sed -n '40,60p' src/core/validation-logic.ts
fiRepository: Fission-AI/OpenSpec
Length of output: 10779
Use resolveOpenSpecDir() to support both openspec/ and .openspec/ directory structures.
Lines 46 and 55 hardcode the 'openspec' path, but other core modules (view-logic.ts, update-logic.ts, list.ts) correctly use resolveOpenSpecDir() from src/core/path-resolver.ts to handle both the legacy openspec/ and default .openspec/ directories. This inconsistency will cause validation to fail for projects using the .openspec/ directory structure. Update the validation module to resolve the OpenSpec directory before constructing paths, matching the pattern used elsewhere in the codebase.
🤖 Prompt for AI Agents
In @src/core/validation-logic.ts around lines 43 - 60, The loops that build
changeDir and file currently hardcode 'openspec' and should use
resolveOpenSpecDir() from path-resolver.ts; call resolveOpenSpecDir() once
(e.g., const openSpecDir = resolveOpenSpecDir()) and replace
path.join(process.cwd(), 'openspec', ...) with path.join(openSpecDir, 'changes',
id) for validateChangeDeltaSpecs and path.join(openSpecDir, 'specs', id,
'spec.md') for validateSpec so the validator.validateChangeDeltaSpecs and
validator.validateSpec calls work for both 'openspec/' and '.openspec/' layouts.
Moved spec logic to src/core/spec-logic.ts and updated src/commands/spec.ts and src/mcp/tools.ts to use it.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/init.ts (1)
756-766: Hardcoded directory path inconsistent with new default.The prompts reference
openspec/project.md, but the PR migrates the default directory to.openspec/. Users initializing fresh will have.openspec/project.mdinstead. Consider usingDEFAULT_OPENSPEC_DIR_NAMEor dynamically resolving the actual path used.Suggested fix
console.log(PALETTE.white('1. Populate your project context:')); console.log( PALETTE.lightGray( - ' "Please read openspec/project.md and help me fill it out' + ` "Please read ${result.openspecPath ?? DEFAULT_OPENSPEC_DIR_NAME}/project.md and help me fill it out` ) );
🤖 Fix all issues with AI agents
In @src/commands/spec.ts:
- Around line 7-13: Replace imports of getSpecIds that currently come from
item-discovery.ts in the four consumer modules and instead import getSpecIds
(and any other spec-related helpers) from spec-logic.ts; specifically update the
import statements in validation-logic, completion-provider, validate command,
and show command to import getSpecIds from '../core/spec-logic.js' (matching how
spec.ts imports getSpecIds) so the code uses resolveOpenSpecDir() logic rather
than the hardcoded openspec/specs path.
In @src/core/change-logic.ts:
- Around line 114-128: Wrap the JSON.parse call for jsonOutput returned by
JsonConverter.convertChangeToJson in a try-catch inside the function that builds
the Change object; catch JSON parsing errors and rethrow or return a more
descriptive error including context like proposalPath and changeName (and
optionally include the raw jsonOutput or the original error message) so callers
can diagnose malformed output from JsonConverter; ensure the rest of the
function (title extraction via extractTitle, id/deltas assignment) only runs
after successful parse.
In @test/core/change-logic.test.ts:
- Around line 77-90: runCreateChange currently only creates the change directory
and metadata, but getActiveChanges expects a proposal.md to mark a change as
active; update runCreateChange to also create a minimal proposal.md (e.g., a
simple "# Change" or basic template) inside the new changeDir so newly
scaffolded changes are immediately discoverable by getActiveChanges; modify the
function that writes metadata in runCreateChange to also write proposal.md to
the same changeDir.
🧹 Nitpick comments (11)
src/commands/init.ts (3)
398-405: Migration prompt may block non-interactive CI environments.The dynamic import of
@inquirer/promptsand call toconfirm()will hang or fail in non-interactive (CI/headless) environments when a legacy directory is detected. Consider adding a fallback for non-interactive mode or exposing a--migrateflag.Suggested approach
if (hasLegacy && !hasDefault) { - const { confirm } = await import('@inquirer/prompts'); - shouldMigrate = await confirm({ - message: `Detected legacy '${LEGACY_OPENSPEC_DIR_NAME}/' directory. Would you like to migrate it to '${DEFAULT_OPENSPEC_DIR_NAME}/'?`, - default: true - }); + if (process.stdin.isTTY) { + const { confirm } = await import('@inquirer/prompts'); + shouldMigrate = await confirm({ + message: `Detected legacy '${LEGACY_OPENSPEC_DIR_NAME}/' directory. Would you like to migrate it to '${DEFAULT_OPENSPEC_DIR_NAME}/'?`, + default: true + }); + } else { + // Non-interactive: default to not migrating, or add a CLI flag + shouldMigrate = false; + } }
432-440: Spinner shows progress for already-completed work.The comments acknowledge this issue:
runInitalready configured the tools, but the spinner suggests work is happening. This creates a misleading UX. Consider either:
- Passing a progress callback to
runInitfor real-time updates- Removing the misleading spinner
- Splitting
runInitto allow step-by-step progress reporting
467-475: Consider movingisToolConfiguredto core logic layer.The comment acknowledges this method should potentially be in the core logic layer. Having this logic in the command layer creates duplication risk if MCP tools need the same functionality. Consider extracting to
src/core/init-logic.tsfor reuse.test/mcp/server.test.ts (1)
4-10: Test coverage is minimal; consider expanding.The test only verifies instantiation. Consider adding tests for:
start()method registration behavior (tools, resources, prompts are registered)- Error handling during startup
Accessing private properties via
as anyis acceptable in tests, but the comment on line 8 could be removed since the cast is self-explanatory.it('can be instantiated', () => { const server = new OpenSpecMCPServer(); expect(server).toBeDefined(); - // accessing private 'server' property is not easy in TS without casting expect((server as any).server).toBeDefined(); });test/mcp/tools.test.ts (2)
1-3: Remove unused importvi.The
viimport from vitest is not used in this test file.-import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach } from 'vitest';
5-11: Consider extracting MockFastMCP to a shared test utility.The same
MockFastMCPpattern is duplicated acrosstools.test.ts,resources.test.ts, andprompts.test.ts. Extract to a shared test helper (e.g.,test/mcp/helpers.ts) to reduce duplication and ensure consistency.test/mcp/resources.test.ts (1)
19-33: Tests verify structure but not load behavior.The tests confirm registration and that
loadfunctions exist, but don't verify the load functions work correctly. Sinceload()depends on file system andprocess.cwd(), integration tests or mocked file system tests would be valuable.Consider adding integration tests or mocked tests that verify:
load()returns correct structure withcontentarray- Error handling when files don't exist
test/mcp/prompts.test.ts (1)
28-45: Good test coverage for prompt content transformation.This test properly verifies that the
toMcpInstructionstransformation replaces CLI commands with MCP tool calls. The assertions checking foropenspec_list_changesandopenspec_validate_change(name: "confirm the replacement logic works.The inline comment (lines 39-41) is verbose; consider simplifying or removing it since the test's intent is clear from the assertions.
// Check for replacement of CLI commands with MCP tools expect(text).toContain('openspec_list_changes'); - expect(text).not.toContain('openspec list'); // Should be replaced/not present as primary instruction ideally, - // but regex replacement might leave some if strictly looking for full command lines. - // The toMcpInstructions function replaces specific patterns. + expect(text).not.toContain('openspec list');src/commands/change.ts (1)
89-89: Remove commented-out dead code.This commented variable declaration should be deleted rather than left with a "// unused now" annotation.
🧹 Proposed fix
- // const changesPath = path.join(await resolveOpenSpecDir(process.cwd()), 'changes'); // unused nowsrc/commands/spec.ts (1)
140-148: Path resolution duplicates logic fromgetSpecMarkdown.The spec existence check (lines 140-145) duplicates the resolution and validation already performed by
getSpecMarkdown. Consider reusing the core function for consistency, though the current approach works correctly.♻️ Alternative using core function
// Validate existence using core function (will throw if not found) - const openspecPath = await resolveOpenSpecDir(process.cwd()); - const specPath = path.join(openspecPath, 'specs', specId, 'spec.md'); - - if (!(await FileSystemUtils.fileExists(specPath))) { - throw new Error(`Spec '${specId}' not found at ${specPath}`); - } + const openspecPath = await resolveOpenSpecDir(process.cwd()); + const specPath = path.join(openspecPath, 'specs', specId, 'spec.md'); + // getSpecMarkdown will throw if spec doesn't exist + await getSpecMarkdown(process.cwd(), specId);src/core/spec-logic.ts (1)
32-52: Consider adding a return type interface forgetSpecJson.The function returns
Promise<any>, but the returned object has a well-defined shape (id, title, overview, requirementCount, requirements, metadata). Adding a return type interface would improve type safety for consumers.♻️ Proposed type definition
export interface SpecJsonOutput { id: string; title: string; overview: string; requirementCount: number; requirements: Array<{ text: string; scenarios: any[] }>; metadata: { version: string; format: string }; } export async function getSpecJson(projectRoot: string, specId: string, options: ShowOptions = {}): Promise<SpecJsonOutput> { // ... existing implementation }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
openspec/changes/archive/2026-01-12-add-mcp-tests/.openspec.yamlopenspec/changes/archive/2026-01-12-add-mcp-tests/proposal.mdopenspec/changes/archive/2026-01-12-add-mcp-tests/specs/mcp-server/spec.mdopenspec/changes/archive/2026-01-12-add-mcp-tests/tasks.mdopenspec/specs/mcp-server/spec.mdsrc/commands/change.tssrc/commands/init.tssrc/commands/spec.tssrc/core/change-logic.tssrc/core/spec-logic.tssrc/mcp/resources.tssrc/mcp/tools.tstest/core/change-logic.test.tstest/mcp/prompts.test.tstest/mcp/resources.test.tstest/mcp/server.test.tstest/mcp/tools.test.ts
✅ Files skipped from review due to trivial changes (3)
- openspec/changes/archive/2026-01-12-add-mcp-tests/.openspec.yaml
- openspec/changes/archive/2026-01-12-add-mcp-tests/specs/mcp-server/spec.md
- openspec/changes/archive/2026-01-12-add-mcp-tests/proposal.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/mcp/resources.ts
- openspec/specs/mcp-server/spec.md
🧰 Additional context used
📓 Path-based instructions (1)
openspec/changes/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Scaffold proposal using
proposal.md,tasks.md, optionaldesign.md, and delta specs underopenspec/changes/<id>/
Files:
openspec/changes/archive/2026-01-12-add-mcp-tests/tasks.md
🧠 Learnings (12)
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
test/core/change-logic.test.tssrc/commands/spec.tssrc/core/spec-logic.tsopenspec/changes/archive/2026-01-12-add-mcp-tests/tasks.mdsrc/commands/change.tssrc/core/change-logic.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
src/commands/spec.tssrc/core/spec-logic.tsopenspec/changes/archive/2026-01-12-add-mcp-tests/tasks.mdsrc/commands/change.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Use `file.ts:42` format for code location references and `specs/auth/spec.md` format for spec references
Applied to files:
src/commands/spec.tssrc/core/spec-logic.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
src/commands/spec.tssrc/core/spec-logic.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Check `openspec/project.md` for project conventions before creating specs
Applied to files:
src/commands/spec.tssrc/core/spec-logic.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Search existing work using `openspec spec list --long`, `openspec list` for enumerating changes; use `rg` only for full-text search
Applied to files:
src/commands/spec.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval
Applied to files:
src/commands/spec.tssrc/commands/change.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
src/core/spec-logic.tsopenspec/changes/archive/2026-01-12-add-mcp-tests/tasks.mdsrc/commands/change.tssrc/commands/init.tssrc/core/change-logic.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
src/core/spec-logic.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
src/core/spec-logic.ts
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
openspec/changes/archive/2026-01-12-add-mcp-tests/tasks.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
openspec/changes/archive/2026-01-12-add-mcp-tests/tasks.mdsrc/commands/init.ts
🧬 Code graph analysis (7)
test/mcp/prompts.test.ts (1)
src/mcp/prompts.ts (1)
registerPrompts(20-62)
test/mcp/tools.test.ts (1)
src/mcp/tools.ts (1)
registerTools(17-414)
test/mcp/server.test.ts (1)
src/mcp/server.ts (1)
OpenSpecMCPServer(10-29)
src/commands/spec.ts (4)
src/core/spec-logic.ts (5)
getSpecMarkdown(23-30)ShowOptions(8-15)getSpecJson(32-52)getSpecIds(54-72)getSpecDetails(74-95)src/utils/interactive.ts (1)
isInteractive(22-28)src/utils/item-discovery.ts (1)
getSpecIds(25-44)src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)
src/mcp/tools.ts (12)
src/core/init-logic.ts (1)
runInit(30-118)src/core/update-logic.ts (1)
runUpdate(18-99)src/core/view-logic.ts (1)
getViewData(16-30)src/core/change-logic.ts (3)
runCreateChange(37-70)getChangeMarkdown(94-102)getChangeJson(104-128)src/core/list.ts (2)
listChanges(55-98)listSpecs(100-127)src/commands/spec.ts (2)
getSpecMarkdown(16-18)getSpecJson(20-22)src/core/spec-logic.ts (2)
getSpecMarkdown(23-30)getSpecJson(32-52)src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/core/validation-logic.ts (1)
runBulkValidation(29-118)src/core/archive-logic.ts (1)
runArchive(31-145)src/core/config-logic.ts (3)
getConfigValue(25-28)setConfigValue(30-61)getConfigList(21-23)src/core/artifact-logic.ts (4)
getArtifactStatus(89-102)getArtifactInstructions(108-139)getApplyInstructions(239-326)getAvailableSchemas(367-369)
src/core/spec-logic.ts (4)
src/commands/spec.ts (2)
getSpecMarkdown(16-18)getSpecJson(20-22)src/core/path-resolver.ts (1)
resolveOpenSpecDir(11-19)src/utils/file-system.ts (1)
FileSystemUtils(44-213)src/core/parsers/markdown-parser.ts (1)
MarkdownParser(10-237)
test/mcp/resources.test.ts (1)
src/mcp/resources.ts (1)
registerResources(6-54)
🔇 Additional comments (17)
src/commands/init.ts (1)
87-372: Well-structured interactive wizard implementation.The
toolSelectionWizardfunction is well-organized with clear step management (intro → select → review), proper keyboard handling, and good separation of concerns. The use ofusePaginationand state hooks from@inquirer/corefollows the library's patterns correctly.test/mcp/tools.test.ts (1)
20-42: Good coverage of tool registration.The test comprehensively verifies all 18 expected tools are registered. The use of
toContainassertions is appropriate for this verification pattern.openspec/changes/archive/2026-01-12-add-mcp-tests/tasks.md (1)
1-29: Task documentation is clear and well-organized.The checklist accurately reflects the implementation work. Based on learnings,
tasks.mdshould ideally use numbered sections (e.g.,## 1. Spec Updates,## 2. Refactoring) for easier reference, though the current format is functional for an archived change.test/core/change-logic.test.ts (2)
19-34: Test setup is correct and comprehensive.The setup correctly simulates a project structure with the legacy
openspec/path (whichresolveOpenSpecDirwill detect and use). The demo change includes bothproposal.mdandtasks.mdas required by the OpenSpec structure.
41-75: Good coverage of core change-logic APIs.Tests verify the essential behaviors: listing changes, retrieving markdown/JSON content, extracting details with task counts, and validation reporting. The assertions are appropriate for the expected return shapes.
src/commands/change.ts (2)
1-13: Clean refactor to core APIs.The migration from in-file parsing logic to the centralized
change-logic.tscore module is well executed. The import surface is minimal and focused.
64-101: List command correctly delegates to core logic.Both short and long formats properly use
getActiveChangesandgetChangeDetailsfrom the core module. The JSON output path correctly gathers details in parallel withPromise.all.src/commands/spec.ts (1)
15-50: SpecCommand class cleanly wraps core logic.The class methods properly delegate to
getSpecMarkdownandgetSpecJsonfrom the core module withprocess.cwd()as the project root. Interactive selection and error handling are appropriate.src/mcp/tools.ts (4)
17-38: Well-structured tool registration with proper Zod 4 patterns.The
openspec_inittool correctly uses Zod 4's.optional().default()chaining for parameters. The error handling pattern withisError: trueis consistent and appropriate for MCP error responses.
140-186: Show tools correctly handle format switching.Both
openspec_show_changeandopenspec_show_specproperly branch onformatparameter to return either markdown text or JSON. The default to JSON aligns with typical MCP tool expectations.
263-326: Config tools expose appropriate configuration API.The config get/set/list tools provide a clean interface for configuration management through MCP. The
setConfigValueoptions (forceString,allowUnknown) give flexibility for advanced use cases.
216-235: Verifyconcurrencyparameter type compatibility.The
concurrencyparameter is defined asz.string()(line 220), but therunBulkValidationfunction may expect a number. Verify the function signature and, if needed, usez.coerce.number()to parse the string or adjust the schema accordingly.src/core/spec-logic.ts (2)
54-72: Robust directory enumeration with proper filtering.The function correctly filters out hidden directories and validates spec existence by checking for
spec.md. The empty array fallback on error is appropriate for listing operations.
97-126: Helper functions are well-implemented.
validateRequirementIndexproperly handles 1-based to 0-based conversion with clear error messaging.filterSpeccorrectly implements the scenarios filtering logic based on therequirementsandscenariosoptions.src/core/change-logic.ts (3)
37-70:runCreateChangecreates directory and metadata only.The function scaffolds the change directory structure and writes metadata but does not create
proposal.mdortasks.md. This is by design (agents/users create content), but consumers should be aware thatgetActiveChangeswon't list the change untilproposal.mdexists.
72-92: Clean implementation of active change enumeration.Properly filters out archive, hidden directories, and directories without
proposal.md. The sorted return provides consistent ordering for consumers.
179-198: Task counting logic is correct.The regex patterns properly identify markdown checkbox syntax (
- [ ],- [x],* [ ],* [x]) with case-insensitivity for thex. The line-by-line iteration is efficient for typical task file sizes.
| import { | ||
| getSpecMarkdown, | ||
| getSpecJson, | ||
| getSpecIds, | ||
| getSpecDetails, | ||
| ShowOptions | ||
| } from '../core/spec-logic.js'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify both files exist
echo "=== Checking for both getSpecIds implementations ==="
find . -name "spec-logic.ts" -o -name "item-discovery.ts" | head -20
# Check if getSpecIds exists in both files
echo -e "\n=== getSpecIds in spec-logic.ts ==="
rg -n "export.*getSpecIds" src/core/spec-logic.ts
echo -e "\n=== getSpecIds in item-discovery.ts ==="
rg -n "export.*getSpecIds" src/utils/item-discovery.ts
# Find all imports of getSpecIds
echo -e "\n=== All imports of getSpecIds ==="
rg -n "getSpecIds" --type ts | grep -E "import|from" | head -20
# Check the implementations to compare
echo -e "\n=== spec-logic.ts getSpecIds implementation ==="
rg -A 10 "export.*function.*getSpecIds|export const getSpecIds" src/core/spec-logic.ts | head -20
echo -e "\n=== item-discovery.ts getSpecIds implementation ==="
rg -A 10 "export.*function.*getSpecIds|export const getSpecIds" src/utils/item-discovery.ts | head -20Repository: Fission-AI/OpenSpec
Length of output: 2113
🏁 Script executed:
#!/bin/bash
# Check what resolveOpenSpecDir does
echo "=== resolveOpenSpecDir in spec-logic.ts ==="
rg -B 2 -A 15 "export.*function.*resolveOpenSpecDir|export const resolveOpenSpecDir" src/core/spec-logic.ts
# Check if .openspec migration context exists
echo -e "\n=== References to .openspec directory ==="
rg "\.openspec" --type ts | head -10
# Check if spec-logic version is used anywhere
echo -e "\n=== Usage of getSpecIds from spec-logic ==="
rg "from.*spec-logic" --type tsRepository: Fission-AI/OpenSpec
Length of output: 1389
🏁 Script executed:
#!/bin/bash
# Search for resolveOpenSpecDir with different patterns
echo "=== Searching for resolveOpenSpecDir ==="
rg "resolveOpenSpecDir" --type ts
# Check the full spec-logic.ts file structure
echo -e "\n=== Functions exported from spec-logic.ts ==="
rg "^export" src/core/spec-logic.ts
# Check imports in spec-logic.ts to understand dependencies
echo -e "\n=== spec-logic.ts imports ==="
head -20 src/core/spec-logic.ts
# Check the project structure to understand openspec/specs vs .openspec/specs
echo -e "\n=== Project root structure ==="
ls -la | grep -E "openspec|\.openspec"
# Check if there's a project.md file as suggested in learnings
echo -e "\n=== openspec/project.md content ==="
cat openspec/project.md 2>/dev/null | head -30Repository: Fission-AI/OpenSpec
Length of output: 5658
🏁 Script executed:
#!/bin/bash
# Check the resolveOpenSpecDir implementation
echo "=== resolveOpenSpecDir implementation ==="
cat src/core/path-resolver.ts
# Check how spec-logic.ts uses it (full context)
echo -e "\n=== spec-logic.ts getSpecIds function (full) ==="
rg -B 2 -A 20 "export async function getSpecIds" src/core/spec-logic.tsRepository: Fission-AI/OpenSpec
Length of output: 1625
Migrate 4 other files from hardcoded item-discovery to dynamic spec-logic version.
The spec.ts import is correct. However, src/core/validation-logic.ts, src/core/completions/completion-provider.ts, src/commands/validate.ts, and src/commands/show.ts still import getSpecIds from item-discovery.ts, which hardcodes the legacy openspec/specs path and won't support the .openspec/ migration. All consumers should import from spec-logic.ts instead, which uses resolveOpenSpecDir() to dynamically resolve both directory structures.
🤖 Prompt for AI Agents
In @src/commands/spec.ts around lines 7 - 13, Replace imports of getSpecIds that
currently come from item-discovery.ts in the four consumer modules and instead
import getSpecIds (and any other spec-related helpers) from spec-logic.ts;
specifically update the import statements in validation-logic,
completion-provider, validate command, and show command to import getSpecIds
from '../core/spec-logic.js' (matching how spec.ts imports getSpecIds) so the
code uses resolveOpenSpecDir() logic rather than the hardcoded openspec/specs
path.
| const converter = new JsonConverter(); | ||
| const jsonOutput = await converter.convertChangeToJson(proposalPath); | ||
| const parsed: Change = JSON.parse(jsonOutput); | ||
| const contentForTitle = await fs.readFile(proposalPath, 'utf-8'); | ||
| const title = extractTitle(contentForTitle, changeName); | ||
| const id = parsed.name; | ||
| const deltas = parsed.deltas || []; | ||
|
|
||
| return { | ||
| id, | ||
| title, | ||
| deltaCount: deltas.length, | ||
| deltas, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Add error handling for JSON parsing.
Line 116 calls JSON.parse(jsonOutput) without error handling. If JsonConverter.convertChangeToJson returns malformed JSON, this will throw an uncaught exception with an unhelpful error message. Consider wrapping in try-catch for a more descriptive error.
🛡️ Proposed fix
const converter = new JsonConverter();
const jsonOutput = await converter.convertChangeToJson(proposalPath);
- const parsed: Change = JSON.parse(jsonOutput);
+ let parsed: Change;
+ try {
+ parsed = JSON.parse(jsonOutput);
+ } catch (e) {
+ throw new Error(`Failed to parse change "${changeName}": invalid JSON output from converter`);
+ }
const contentForTitle = await fs.readFile(proposalPath, 'utf-8');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const converter = new JsonConverter(); | |
| const jsonOutput = await converter.convertChangeToJson(proposalPath); | |
| const parsed: Change = JSON.parse(jsonOutput); | |
| const contentForTitle = await fs.readFile(proposalPath, 'utf-8'); | |
| const title = extractTitle(contentForTitle, changeName); | |
| const id = parsed.name; | |
| const deltas = parsed.deltas || []; | |
| return { | |
| id, | |
| title, | |
| deltaCount: deltas.length, | |
| deltas, | |
| }; | |
| } | |
| const converter = new JsonConverter(); | |
| const jsonOutput = await converter.convertChangeToJson(proposalPath); | |
| let parsed: Change; | |
| try { | |
| parsed = JSON.parse(jsonOutput); | |
| } catch (e) { | |
| throw new Error(`Failed to parse change "${changeName}": invalid JSON output from converter`); | |
| } | |
| const contentForTitle = await fs.readFile(proposalPath, 'utf-8'); | |
| const title = extractTitle(contentForTitle, changeName); | |
| const id = parsed.name; | |
| const deltas = parsed.deltas || []; | |
| return { | |
| id, | |
| title, | |
| deltaCount: deltas.length, | |
| deltas, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In @src/core/change-logic.ts around lines 114 - 128, Wrap the JSON.parse call
for jsonOutput returned by JsonConverter.convertChangeToJson in a try-catch
inside the function that builds the Change object; catch JSON parsing errors and
rethrow or return a more descriptive error including context like proposalPath
and changeName (and optionally include the raw jsonOutput or the original error
message) so callers can diagnose malformed output from JsonConverter; ensure the
rest of the function (title extraction via extractTitle, id/deltas assignment)
only runs after successful parse.
| it('runCreateChange scaffolds a new change', async () => { | ||
| const newChangeName = 'new-test-change'; | ||
| const result = await runCreateChange(tempRoot, newChangeName); | ||
|
|
||
| expect(result.name).toBe(newChangeName); | ||
| expect(result.changeDir).toContain(newChangeName); | ||
|
|
||
| // Manually create proposal.md as getActiveChanges requires it | ||
| await fs.writeFile(path.join(result.changeDir, 'proposal.md'), '# Change', 'utf-8'); | ||
|
|
||
| // Verify file creation | ||
| const changes = await getActiveChanges(tempRoot); | ||
| expect(changes).toContain(newChangeName); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the runCreateChange implementation
rg -nA 15 'export.*runCreateChange|function runCreateChange' src/core/change-logic.tsRepository: Fission-AI/OpenSpec
Length of output: 590
🏁 Script executed:
# Check what getActiveChanges requires/expects
rg -nA 20 'export.*getActiveChanges|function getActiveChanges' src/core/change-logic.tsRepository: Fission-AI/OpenSpec
Length of output: 917
🏁 Script executed:
# Look for other usages of runCreateChange in tests to understand expected behavior
rg -n 'runCreateChange' test/ --type tsRepository: Fission-AI/OpenSpec
Length of output: 312
🏁 Script executed:
# Check if there's any documentation about the change scaffolding workflow
fd -t f '\.(md|txt)$' . | xargs rg -l 'runCreateChange|scaff' | head -10Repository: Fission-AI/OpenSpec
Length of output: 624
🏁 Script executed:
# Get the complete runCreateChange implementation
sed -n '37,70p' src/core/change-logic.tsRepository: Fission-AI/OpenSpec
Length of output: 938
🏁 Script executed:
# Also check if there's a proposal.md template or default content anywhere
rg -n 'proposal.md' src/ --type ts -B 2 -A 2Repository: Fission-AI/OpenSpec
Length of output: 20678
🏁 Script executed:
# Look for any initialization/scaffolding logic in the change-logic file around runCreateChange
sed -n '37,100p' src/core/change-logic.tsRepository: Fission-AI/OpenSpec
Length of output: 2154
🏁 Script executed:
# Check if there are any comments or TODOs about scaffolding proposal.md in runCreateChange
rg -n 'TODO|FIXME|scaff' src/core/change-logic.ts | head -20Repository: Fission-AI/OpenSpec
Length of output: 45
🏁 Script executed:
# Look at the metadata writing to understand if there's a pattern for scaffold templates
rg -nA 10 'writeChangeMetadata|writeMetadata' src/core/change-logic.tsRepository: Fission-AI/OpenSpec
Length of output: 820
runCreateChange creates an incomplete scaffold that getActiveChanges cannot discover.
The function creates only the directory and metadata but not proposal.md, which getActiveChanges explicitly requires (line 80 checks for its presence and skips directories without it). This leaves newly created changes invisible to change discovery until proposal.md is manually created—as your test demonstrates at line 85.
Either scaffold a minimal proposal.md template during creation, or clearly document this as intended behavior where users/agents must manually create the proposal content before changes become discoverable.
🤖 Prompt for AI Agents
In @test/core/change-logic.test.ts around lines 77 - 90, runCreateChange
currently only creates the change directory and metadata, but getActiveChanges
expects a proposal.md to mark a change as active; update runCreateChange to also
create a minimal proposal.md (e.g., a simple "# Change" or basic template)
inside the new changeDir so newly scaffolded changes are immediately
discoverable by getActiveChanges; modify the function that writes metadata in
runCreateChange to also write proposal.md to the same changeDir.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openspec/specs/cli-init/spec.md (1)
22-34: Internal inconsistency: directory references not fully updated.The spec now specifies
.openspec/as the directory structure (lines 22, 28), but numerous unchanged references throughout the document still point toopenspec/:
- Line 41:
openspec/AGENTS.md- Lines 88, 95-96:
@/openspec/AGENTS.mdin stub template- Lines 114, 150:
openspec/directory checks- Line 125: prompt references
openspec/AGENTS.mdWhile the new Legacy Migration requirement (lines 326-336) addresses migrating from
openspec/to.openspec/, the spec's own examples and templates should reflect the new canonical location for forward-looking scenarios.
🤖 Fix all issues with AI agents
In @README.md:
- Around line 157-172: Clarify the Gemini CLI Extension docs: update the "Zero
Configuration" bullet to specify it only applies to automatic slash-command
setup (not to requiring the OpenSpec CLI), expand "Auto-Maintenance" to briefly
describe/point to GEMINI.md for details about automatic CLI version checks,
slash command generation, and command sync, and add a short compatibility
section explaining that gemini-extension.json and package.json must match (what
happens if they drift, how to verify versions, minimum supported version), fix
the current repo mismatch by aligning package.json and gemini-extension.json
versions, and verify/confirm the install command format (gemini extensions
install <repo-url>) is correct before keeping it.
🧹 Nitpick comments (5)
test/core/init.test.ts (4)
7-8: Remove unused imports.
FileSystemUtilsandToolRegistryare imported but never used in this test file.Suggested fix
import { runInit } from '../../src/core/init-logic.js'; import { InitCommand } from '../../src/commands/init.js'; -import { FileSystemUtils } from '../../src/utils/file-system.js'; -import { ToolRegistry } from '../../src/core/configurators/registry.js';
728-737: Consider adding a test for migration decline scenario.The migration test only covers the
shouldMigrate: truecase. Consider adding a test that verifies behavior whenshouldMigrate: false(or omitted) with a legacy directory present — the system should continue using the legacy path without migrating.Suggested additional test
it('should use legacy directory without migration when shouldMigrate is false', async () => { const legacyPath = path.join(tempDir, 'openspec'); await fs.mkdir(legacyPath, { recursive: true }); const result = await runInit(tempDir, { tools: [], shouldMigrate: false }); expect(result.migrated).toBe(false); expect(result.openspecDir).toBe('openspec'); // Legacy directory should still exist, default should not expect(await fileExists(legacyPath)).toBe(true); expect(await fileExists(path.join(tempDir, '.openspec'))).toBe(false); });
12-16: Consider organizing tests by testing approach.The describe block is named
runInit, but most tests exerciseInitCommand.execute(). Consider using nested describe blocks to separate:
- Direct
runInittests (core logic)InitCommandintegration tests (CLI command wrapper)This would improve clarity about what each test verifies.
68-72: Consider usingfileExistshelper for consistency.These assertions use
fs.stat(...).toBeDefined()while other tests in this file use the localfileExistshelper. Using the helper consistently would improve readability.Suggested change
const openspecPath = path.join(tempDir, '.openspec'); - expect(await fs.stat(openspecPath)).toBeDefined(); - expect(await fs.stat(path.join(openspecPath, 'specs'))).toBeDefined(); - expect(await fs.stat(path.join(openspecPath, 'changes'))).toBeDefined(); - expect(await fs.stat(path.join(openspecPath, 'project.md'))).toBeDefined(); + expect(await fileExists(openspecPath)).toBe(true); + expect(await fileExists(path.join(openspecPath, 'specs'))).toBe(true); + expect(await fileExists(path.join(openspecPath, 'changes'))).toBe(true); + expect(await fileExists(path.join(openspecPath, 'project.md'))).toBe(true);test/core/update.test.ts (1)
11-13: Describe block name inconsistent with test implementations.The describe block is named
'runUpdate', but only 2 tests (lines 38-42, 44-56) actually invokerunUpdate()directly. All other tests continue to useupdateCommand.execute().Consider either:
- Renaming the describe block to reflect both APIs being tested, or
- Migrating remaining tests to use
runUpdatedirectly ifUpdateCommandis being deprecated.If testing both the core logic (
runUpdate) and the command wrapper (UpdateCommand.execute()) is intentional, consider using nested describe blocks for clarity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.github/workflows/release-prepare.ymlREADME.mdopenspec/specs/cli-init/spec.mdpackage.jsonsrc/cli/index.tssrc/commands/artifact-workflow.tssrc/core/config.tssrc/utils/file-system.tstest/core/init.test.tstest/core/update.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- package.json
- src/commands/artifact-workflow.ts
- src/core/config.ts
- .github/workflows/release-prepare.yml
🧰 Additional context used
📓 Path-based instructions (1)
openspec/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Files:
openspec/specs/cli-init/spec.md
🧠 Learnings (11)
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
openspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
openspec/specs/cli-init/spec.mdREADME.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
openspec/specs/cli-init/spec.mdREADME.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
openspec/specs/cli-init/spec.mdREADME.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
openspec/specs/cli-init/spec.mdREADME.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Check `openspec/project.md` for project conventions before creating specs
Applied to files:
openspec/specs/cli-init/spec.mdREADME.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
openspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
openspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Use `openspec archive <change-id> --skip-specs --yes` for tooling-only changes
Applied to files:
openspec/specs/cli-init/spec.md
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Always open `@/openspec/AGENTS.md` when the request mentions planning or proposals (words like proposal, spec, change, plan), introduces new capabilities, breaking changes, architecture shifts, or performance/security work, or sounds ambiguous and needs the authoritative spec before coding
Applied to files:
openspec/specs/cli-init/spec.mdREADME.md
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Search existing work using `openspec spec list --long`, `openspec list` for enumerating changes; use `rg` only for full-text search
Applied to files:
README.md
🧬 Code graph analysis (3)
test/core/update.test.ts (1)
src/core/update-logic.ts (1)
runUpdate(18-99)
test/core/init.test.ts (2)
src/utils/file-system.ts (1)
fileExists(88-98)src/core/init-logic.ts (1)
runInit(30-118)
src/cli/index.ts (1)
src/commands/serve.ts (1)
ServeCommand(3-8)
🔇 Additional comments (11)
src/utils/file-system.ts (1)
84-87: LGTM - follows existing class patterns.The method is a clean wrapper consistent with other write operations in this class. One consideration:
fs.renamefails withEXDEVfor cross-device moves (paths on different filesystems). If this is used for the.openspecdirectory migration mentioned in the PR, this is unlikely to be an issue for typical same-root renames, but callers should be aware of this limitation.test/core/update.test.ts (3)
38-42: LGTM!This test correctly validates the failure path when neither
openspecnor.openspecdirectory exists.
44-56: LGTM!This test correctly validates the
.openspecdirectory migration path and verifies therunUpdateresult structure.
31-36: LGTM!The
vi.restoreAllMocks()call ensures test isolation by cleaning up all mocks after each test.openspec/specs/cli-init/spec.md (2)
318-324: LGTM!The new safety requirement correctly uses SHALL for normative language and provides actionable guidance for CLI availability checks. This aligns with PR objectives for extension support where users may install the extension before the CLI.
326-336: LGTM!The legacy migration requirement properly uses SHALL and follows the GIVEN/WHEN/THEN scenario format. This addresses the transition from the old
openspec/directory to the new.openspec/hidden directory, which aligns with the PR objectives for reducing project clutter.src/cli/index.ts (3)
7-18: LGTM!The import reorganization consistently moves command implementations from
../core/to../commands/, aligning with the architectural refactor to separate CLI adapters from core logic. The newServeCommandimport follows the same pattern.
104-104: LGTM!The dynamic import path update is consistent with the static import reorganization.
131-144: LGTM!The new
servecommand is well-implemented:
- Uses
console.errorinstead ofora().fail()to avoid contaminating stdout, which is appropriate for MCP stdio transport.- Error handling pattern is consistent with other commands.
- The global
preActionhook will still apply for telemetry tracking.README.md (2)
132-132: LGTM!The table update appropriately signals the new native extension capability for Gemini CLI users.
88-108: [Your rewritten review comment text here]
[Exactly ONE classification tag]
| ### Gemini CLI Extension (Native) | ||
|
|
||
| OpenSpec is available as a native extension for the [Gemini CLI](https://geminicli.com). This provides deep contextual awareness and native slash commands without manual configuration. | ||
|
|
||
| **Install the extension:** | ||
| ```bash | ||
| gemini extensions install https://github.com/Fission-AI/OpenSpec | ||
| ``` | ||
|
|
||
| **Benefits:** | ||
| - **Zero Configuration**: Automatically sets up `/openspec` slash commands. | ||
| - **Native Context**: Gemini becomes "OpenSpec-aware" instantly. | ||
| - **Auto-Maintenance**: The agent can self-repair its command definitions from the source of truth. | ||
|
|
||
| *Note: You still need the [OpenSpec CLI](#step-1-install-the-cli-globally) installed globally for the agent to perform operations.* | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's read the actual README.md to confirm the snippet
echo "=== README.md lines 157-172 ==="
sed -n '157,172p' README.md
echo -e "\n=== Searching for Gemini CLI extension format documentation ==="
rg -n "gemini extensions install" -i
echo -e "\n=== Searching for version synchronization mentions ==="
rg -n "version.*sync|sync.*version|extension.*version" -i
echo -e "\n=== Searching for extension manifest or configuration ==="
fd -e json -e yml -e yaml "gemini.*extension|extension.*gemini"
echo -e "\n=== Searching for geminicli.com references ==="
rg -n "geminicli.com"
echo -e "\n=== Listing root level config files ==="
ls -la | grep -E "\.(json|yml|yaml|toml|config)"
echo -e "\n=== Searching for auto-maintenance or self-repair references ==="
rg -n "self-repair|auto-maintain|auto-update" -iRepository: Fission-AI/OpenSpec
Length of output: 5647
🏁 Script executed:
#!/bin/bash
echo "=== gemini-extension.json content ==="
cat gemini-extension.json
echo -e "\n=== sync-extension-version.mjs script ==="
cat scripts/sync-extension-version.mjs
echo -e "\n=== check-extension-version.mjs script ==="
cat scripts/check-extension-version.mjs
echo -e "\n=== CI Sync specification ==="
cat openspec/specs/ci-sync/spec.md
echo -e "\n=== Searching for any gemini CLI documentation or setup ==="
fd "gemini" -t f | head -20
echo -e "\n=== Package.json dependencies related to gemini ==="
jq '.dependencies, .devDependencies' package.json | grep -i geminiRepository: Fission-AI/OpenSpec
Length of output: 2332
🏁 Script executed:
#!/bin/bash
echo "=== GEMINI.md content ==="
cat GEMINI.md
echo -e "\n=== package.json version field ==="
jq '.version' package.json
echo -e "\n=== Searching for any references to extension installation or gemini CLI format ==="
rg -n "install.*extension|extensions.*install" -i
echo -e "\n=== Searching for documentation about version requirements ==="
rg -n "require.*version|version.*require|compatibility" -i README.mdRepository: Fission-AI/OpenSpec
Length of output: 7731
Address conflicting documentation and critical version compatibility gaps.
The extension section contains several issues that need resolution:
-
"Zero Configuration" contradicts explicit requirements (lines 167 vs 171): The benefits claim "Zero Configuration," but line 171 explicitly requires the OpenSpec CLI installed globally. Either remove "Zero Configuration" or clarify that it applies only to slash command setup, not CLI installation.
-
"Auto-Maintenance" lacks explanation (line 169): This term is unexplained. The related documentation in
GEMINI.mddescribes automatic CLI version checking, slash command generation, and command definition synchronization from source files—but the README provides no clarity or reference to this context. Add a brief explanation or link to the detailed auto-initialization logic. -
Missing version compatibility guidance: The CI infrastructure (scripts and specs) enforces strict version synchronization between
gemini-extension.jsonandpackage.json. However, the README provides zero guidance on this critical operational concern:- What happens when extension and CLI versions drift?
- How do users verify compatibility?
- Are there minimum version requirements?
Additionally, the current repository state shows a version mismatch (
package.json: 0.19.0vsgemini-extension.json: 0.18.0), which contradicts the documented CI requirement that these must always match. This needs to be resolved and documented. -
Installation command format not verified: Confirm that
gemini extensions install https://github.com/Fission-AI/OpenSpecis the correct format for Gemini CLI extension installation.
🤖 Prompt for AI Agents
In @README.md around lines 157 - 172, Clarify the Gemini CLI Extension docs:
update the "Zero Configuration" bullet to specify it only applies to automatic
slash-command setup (not to requiring the OpenSpec CLI), expand
"Auto-Maintenance" to briefly describe/point to GEMINI.md for details about
automatic CLI version checks, slash command generation, and command sync, and
add a short compatibility section explaining that gemini-extension.json and
package.json must match (what happens if they drift, how to verify versions,
minimum supported version), fix the current repo mismatch by aligning
package.json and gemini-extension.json versions, and verify/confirm the install
command format (gemini extensions install <repo-url>) is correct before keeping
it.
|
@TabishB You were right, this approach is too clunky. I did some research and implemented an additional mcp tool that allows any mcp supporting agent(most now) to integrate openspec natively without having to install the package directly(using npx now to serve the client). Ill close this pr, the new proposal is in #488 |
2 participants
Native Gemini CLI integration and unified prompts extracted to templates. When added as a gemini extension, the extension will guide the user through its own installation and
openspecfolder and slash commands creation. Once merged we can add OpenSpec to gemini's extensions board https://geminicli.com/extensions/Example onboarding during agent workflow for adding the first spec when Openspec not installed:
Gemini extensions ink:
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.